* fix(scan): match --path selectors case-insensitively
On Windows and macOS a --path selector such as SRC names an existing directory (the filesystem resolves it), but filterByPaths compared selectors to git ls-files output byte-for-byte, so the scan silently selected no files and exited 0. Fold case like the other user-facing path filters (--include/--exclude, secret paths, the extension allowlist).
Refs #1708
* fix(scan): prefer the exact --path spelling before folding case
A selector now matches its own spelling first, so on a case-sensitive
filesystem --path src cannot widen to a sibling SRC/; case-insensitive
matching stays as a per-selector fallback, the repair for --path SRC on
Windows and macOS. Documented on the ocr scan page in all locales.
* fix(scan): resolve --path against the filesystem before folding case
A selector's own spelling is tried first; a spelling that names nothing
must resolve in the working tree before case folding applies, so on a
case-sensitive filesystem a wrongly-cased --path is an error instead of
a folded match or a silent empty scan. Docs updated in all locales.
* fix(scan): resolve --path selectors to their on-disk path
A selector that names nothing in its own spelling is resolved against
the working tree -- symlinks followed, on-disk case taken where the
filesystem folds it -- and only the resolved path is matched, replacing
the case-fold fallback. An empty src/docs/ therefore selects nothing on
a case-sensitive filesystem even while src/DOCS/ holds tracked files.
---------
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
* docs: nushell.md from @hustcer
* feat: file extensions
* chore: add new nu (NU) extensions
* Credits for `nushell.md`
Co-authored-by: hustcer <hustcer@outlook.com>
* docs: update supported languages
* fixup: CI failures from bad merge
* chore: fix merge
* credits for nushell.md
* docs: sync nushell into korean site docs
* chore: append excludsion to indexes
* chore: format with prettier [skip ci]
---------
Co-authored-by: hustcer <hustcer@outlook.com>
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
- Enhanced the contributing documentation by adding new sections for `acp`, `docs`, `imgs`, `chatgptauth`, `delegate`, `llmloop`, `mcp`, `scan`, and `scripts`.
- Improved clarity in existing sections, particularly in `cmd/opencodereview/`, emphasizing the role of command dispatch.
- Ensured consistency in terminology and descriptions across English, Japanese, Korean, Russian, and Chinese versions of the documentation.
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
Keep the CONTRIBUTING Project Structure trees in sync with main,
including internal/chatgptauth from #1622 (review feedback on #1721).
Co-authored-by: Frank-zhu2000 <332350688+Frank-zhu2000@users.noreply.github.com>
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
* fix(scan): surface llmloop token-budget stop on the run-level state
`#1248` (merged 2026-09-17) added an in-conversation token-budget stop to
`internal/llmloop.RunMainTask` and wired its `StopTokenBudget` outcome
through to the review path's run-level `budgetExceeded` flag plus a
`token_budget_reached` warning (see `internal/agent/agent.go` near
`executeGroupSubtask`).
The scan path was left out: `internal/scan.Agent.budgetExceeded` is
still set only by the dispatchBatch look-ahead check, and
`executeSubtask` does not react to a `StopTokenBudget` returned by
`llmloop.RunMainTask`. The result is that an in-flight token-budget
stop fired mid-task on a scan run leaves:
- `cmd/opencodereview` `--format json` `budget_exceeded: false` even
though the run stopped on the budget, and the stderr
`budget_exceeded=...` line lying for the same reason;
- no `token_budget_reached` warning, so `ocr session list` and the
other warnings-driven surfaces can't distinguish a budget stop from
other failure modes.
This change mirrors the review path: in `executeSubtask`, on a
non-completed stop with `stop == llmloop.StopTokenBudget`, atomically
flip `budgetExceeded` via `CompareAndSwap` and emit one
`token_budget_reached` warning. `budgetExceeded` is now
`atomic.Bool` (was plain `bool`) for the same reason the review
agent uses one.
Fixes the gap between the two commands' `BudgetExceeded()` /
`token_budget_reached` reporting. The per-item failure record keeps
existing semantics: the inner `main_task did not complete before
stopping: <Reason>` string still feeds the `scan_subtask_error`
warning unchanged, so the diagnostic that survives `--format json`
continues to name the stop trigger via `llmloop.StopTokenBudget.Reason()`.
* docs: Apply batched suggestions from code review
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
---------
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
* fix(llmloop): count a grace-round task_done as completion
The grace round after an exhausted budget offers only code_comment and
task_done, and asks the model to call task_done when it has nothing further
to report. That checkpoint was discarded and RunMainTask returned
completed=false, so a file the model had finished was recorded as
incomplete: scan wrote review_item_failed instead of a reusable
review_item_done, and a later resume re-ran it.
Report grace-round completion and return it. Stops that never run a grace
round, and a task_done(FAILED), are unchanged.
closes#1018
* fix(llmloop): warn when a review limit reaches the grace round
A task that spends its tool-request rounds or token budget and then
completes in the grace round reported success with no trace of the
limit, so a zero-finding result could read as clean. Record a
review_limit_reached warning with the stop reason and the round's
verdict, and qualify the zero-finding message in text and JSON output.
Completion, the reusable checkpoint, and budget_exceeded are unchanged.
* fix(llmloop): let failure veto grace-round completion
---------
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
* 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
* feat(auth): enable ChatGPT plan reviews through public Responses
* fix(auth): prevent stale credential status and provider keys
* Fix ChatGPT auth tests on Windows
---------
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
Add an independent stdio ACP adapter with slash commands, optional
natural-language parsing, managed review/scan processes, CI, and
multilingual setup docs. Keep the OCR CLI source and prompts unchanged.
* refactor(acp): add ACP adapter and supporting services
* ci(acp): use patched Go toolchain
* docs(acp): trim support caveats and sync CI notes
Drop dated verification claims and the prebuilt-binary disclaimer so the
adapter docs match the current local-build story and acp-ci.yml.
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
`ocr scan --path .` selected no files and exited 0. `git ls-files` prints
repository-relative names such as `main.go`, and filterByPaths matches an
exact path or a `<selector>/` prefix, so a selector left as "." matched
nothing. `--path ./` normalized to the same dead selector. Both spellings now
mean the whole repository, which is what an omitted --path already means.
filepath.ToSlash also ran after the "./" trim. On Windows a selector like
`.\internal\scan` only becomes `./internal/scan` during that conversion, so
the prefix survived the trim and matched nothing there either. ToSlash now
runs first, and the "./" trim repeats so `././` collapses.
A root selector alongside a narrower one widens the scan, since the root
already covers it.
closes#1596closes#1597
* fix(llm): let --timeout raise the per-request HTTP deadline
The --timeout flag documents a concurrent-task timeout in minutes, but
the per-request HTTP deadline stayed at the client's independent
5-minute default. A review of one slow file therefore failed with
classification=timeout at duration_to_headers_ms≈300000 even when the
caller granted --timeout 30, and no CLI setting could let the file
review finish (issue #1155).
ResolveOptions gains a TaskTimeout field: when no env
(OCR_LLM_TIMEOUT) or config-file (timeout_sec) request timeout is set,
the task budget raises the request deadline so a single request can use
the whole task budget. Existing precedence is unchanged — env and
config-file timeouts still win, and leaving --timeout at its default
keeps the 5-minute client default.
ocr review and ocr scan both pass their --timeout value through.
Fixes#1155
* fix(cmd): feed the concurrent task timeout into the request deadline
main replaced the per-file timeout flag with --timeout binding
concurrentTaskTimeout, and the agent applies it as a context deadline
across review rounds — but the per-request HTTP deadline stayed at the
client's independent 5-minute default, so a single slow file request
could still die inside a task that had plenty of budget left (#1155's
original symptom).
Wire ResolveOptions.TaskTimeout from the live
opts.concurrentTaskTimeout (minutes) in both review and scan commands.
Precedence is unchanged: env OCR_LLM_TIMEOUT and config-file
timeout_sec still win, and an unset task timeout keeps the 5-minute
client default.
* fix(llm): keep request and task timeouts independent
* docs: localize timeout configuration guidance
* fix(llm): avoid attributing caller deadlines to task timeout
---------
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
* fix(idea): list merge commit files against the first parent
* test(idea): skip the merge commit listing when git is not on PATH
The test drives a real git binary. The CI runner's test JVM has none on
its PATH, so ProcessBuilder failed before the first command ran.
* fix(make): append .exe to the dev binary on Windows
make build on Windows wrote dist/opencodereview without an extension, which cmd and PowerShell cannot launch. Build, run, and help now use a .exe extension when OS is Windows_NT.
* docs: name the Windows .exe output of make build
The pager hides rows via the hidden attribute, but .file-list li's
author display: flex outranks the UA [hidden] rule, so every page's
rows stayed visible. Add a .file-list li[hidden] guard like the pager
controls already have for .pagination[hidden], and extend the guard
test to cover row-level [hidden] rules.
Fixes#1575
Assistant turns issuing tool calls under OpenAI-compatible chat completions store their calls and arguments on ToolCalls rather than Native.Payload. Because messageTokens only inspected ExtractText() and Native.EstimatedTokens(), these turns evaluated to 0 tokens during active zone budget calculations, causing computeActiveZoneSize to pack too many rounds and risk exceeding model context limits.
Adds EstimatedTokens to ToolCall (including arguments and ExtraContent) and Message, and updates messageTokens to account for tool calls when not already covered by Native payload.
* docs(cli-reference): list the manifest status values in the status field
The status field table listed only success / completed_with_warnings /
completed_with_errors / skipped. Those four are the pre-manifest path;
every review run that produced a manifest reports the manifest's terminal
state instead (complete / partial / failed / skipped), so the documented
enum was missing the values an actual run emits. Document both paths and
note that skipped also covers the no-supported-files case. Synced across
all five locales.
* docs: Apply batched suggestions from code review
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
---------
Co-authored-by: ydflow <314143294+ydflow@users.noreply.github.com>
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
* fix(llm): trim environment-sourced endpoint values
A trailing CR in an environment value — a .env file or a shell wrapper written
with CRLF line endings is the usual source — reached url.Parse and the auth
header verbatim, so one stray CR failed every file of a review with an opaque
error that named neither the environment nor the variable.
Trimming in finalizeResolvedEndpoint instead is not enough: tryCCEnv calls
ensureMessagesSuffix, which appends "/v1/messages" and buries the CR in the
middle of the URL where a later TrimSpace can no longer reach it.
Closes#1395
* Improve LLM endpoint diagnostics
A single request never covers the turn after a tool call, so a provider that
rejects that turn passed the connectivity test while failing every review.
Refs #1357
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
Pass -c core.quotepath=false to git ls-files and git ls-tree, matching code_search and diff, so file_find reports paths that match queries and can be opened by file_read.
* feat(llm): add OpenRouter as a built-in provider
* feat: Apply suggestion from @lizhengfeng101
* test: drop openrouter_test.go
OpenRouter is a standard ProtocolOpenAIChatCompletions entry — no new
protocol, no special auth, no new code path. Every previous provider
addition in the same category (gemini, xAI, kimi-global, siliconflow,
novita) only touched providers_test.go, which this PR already does.
TestResolveEndpoint_OpenRouter re-tests generic resolver logic already
covered by resolver_test.go. TestOpenRouter_ToolCallingRequest re-tests
the OpenAI chat completions client already covered by client_test.go.
The TestLookupProvider_OpenRouterDetails + sorted-list update in
providers_test.go is sufficient.
---------
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
* docs(review-rules): document merge_system_rule in all five locales
merge_system_rule has existed on a rule.json entry for a while, but the
review rules guide never mentioned it. The only place it appeared was
skills/open-code-review/SKILL.md, which most users never read. The cost
is real: #1132 asked for exactly this capability as a new feature,
because the docs give every indication that a catch-all user rule
unavoidably discards the per-language system rules.
The field list now carries it alongside path and rule, with the default
stated. A worked example shows the JSON and the resulting
`ocr rules check` output so the System-Specific / User-Specific halves
are visible, and the two edge cases from mergeWithSystemRule are called
out: either half may be empty, and the other is not replaced by a
placeholder.
The system half is resolved per file, so one catch-all entry yields
java.md for .java and python.md for .py. That is the property that makes
the field worth documenting, and it is the one a reader would otherwise
assume away.
Also states the limitation: merging reaches the system layer only. Two
user entries matching the same file still resolve first-match-wins, and
a matching layer still shadows the ones below it.
A recipe covers the common case, since "global security rules on top of
the built-in per-language rules" is what people search for.
Docs only, in en, zh, ja, ru and ko. The translated pages are exempt
from the English-only scan by extension.
* docs(review-rules): address review feedback on merge_system_rule pages
- en: fix the grammar in the per-file resolution bullet and state that
the global rule file is the lowest of the three user layers, so a
matching --rule or project entry shadows it
- zh/ja/ko/ru: carry the same layer-priority caveat into each recipe
- ja: fix the 置き換えます typo and reword two sentences that were
literal translations of the English
---------
Co-authored-by: ydflow <314143294+ydflow@users.noreply.github.com>
Sessions accumulate in ~/.opencodereview/sessions/ with no way to remove one
short of deleting files by hand. The Viewer stays read-only by design, so
deletion belongs in the CLI, where the caller already owns the files.
The session id is enough on its own: the command looks for it under every
repository, so it works from anywhere. That a session is stored under a
directory derived from its repository is an implementation detail, and making
the caller stand in the right repository to delete by id pushes that detail
onto them. --repo narrows the search rather than enabling it.
An id saved for more than one repository lists the candidates and deletes
nothing. encodeRepoPath replaces separators with "-", so two repository paths
can share one sessions directory and one id can name two different files;
choosing between them would be a coin flip on a file that cannot be recovered.
The command prints the session's repository, branch, start time, file count
and comment count in the same format `ocr session list` uses, then asks to
confirm. A non-interactive stdin answers no, so `--yes` has to be passed
deliberately from a pipeline or CI.
Reports an unreadable session rather than folding it into "corrupt" -- a
session whose metadata cannot be parsed is still deletable, since that is the
record most likely to need removing. Documented in all five locales of the CLI
reference.
Co-authored-by: Qiyuanqiii <267806965+Qiyuanqiii@users.noreply.github.com>
Co-authored-by: wu21-web <wu2196674@icloud.com>
* fix(llm): replay tool_call extra_content and surface provider error bodies
Gemini 3 on OpenAI-compatible endpoints requires the thought signature in
tool_calls[].extra_content to be echoed back unchanged, and array-wrapped
error documents were dropped before reaching the user.
Fixes#1357Fixes#1042
* feat: Apply suggestion from @lizhengfeng101
* fix(session): keep llm_request tool-call field order stable
Project tool calls through an ordered struct instead of a map, so records
without extra_content stay byte-identical to the previous format.
* fix(llm): drop invalid UTF-8 from surfaced error bodies
A character split by the 64 KiB display cap became a replacement
character, which the source also carried as a literal.
Co-authored-by: Tao Xin <wu2196674@icloud.com>
---------
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
Co-authored-by: Tao Xin <wu2196674@icloud.com>
* security: validate credential commands and warn on plaintext secrets
- Add validateKeyCmd() in internal/llm/keycmd.go to reject suspicious
shell patterns (command separators, backticks, redirections) in
api_key_cmd and auth_token_cmd before the shell executes them.
Provides a clear diagnostic for tampered or accidentally malformed
config rather than silently running destructive expressions.
- Add plaintext credential warnings in internal/llm/resolver.go when
api_key or auth_token are stored as static strings in the config
file. Credential is still accepted; the warning nudges users toward
api_key_cmd / auth_token_cmd so secrets are not at rest in JSON.
- Extend .gitattributes with explicit binary markers for web fonts
(woff, woff2, ttf, otf, eot), WebAssembly (.wasm), compressed
archives (.zip, .tar, .gz, .tgz), and other image formats (.webp,
.bmp, .tiff) to prevent CRLF corruption on Windows clones.
* fix(keycmd): tighten validateKeyCmd per review feedback
Three bugs flagged by the code review bot:
- Backtick regex was backtick-space, missing the common case of`cmd`
with no space. Changed to match any backtick character.
- Command-separator [;|&] and redirection \d*>>?\s*\S were too broad:
they rejected existing tests that use semicolons, ampersands, and
2>/dev/null, which are legitimate in helpers that suppress stderr
or chain commands. Both patterns are removed; only backtick and
null byte checks remain, each with zero plausible false-positives.
- Custom min() shadowed Go built-in min (Go 1.21+). go.mod declares
go 1.25.5, so removed the custom helper and inlined the bounds check.
* security: gate plaintext credential warnings and add tests
Address review feedback on the credential-hardening change:
- Emit the plaintext api_key / auth_token warning only when no
corresponding api_key_cmd / auth_token_cmd is configured. This removes
the duplicate warning when both a static key and a command were set,
and limits the nudge to configs that actually lack a secret-manager
path. The legacy check now also runs after the completeness guard so
an incomplete llm block no longer warns spuriously.
- Trim the oversized suspiciousPatterns / validateKeyCmd comments to the
essentials.
- Add tests: validateKeyCmd backtick / NUL rejection (unit and through
resolveKeyCmd), and the plaintext-warning fire / suppress paths for
both the provider and legacy config.
* Revert plaintext-credential warning from #1291
The nudge printed a stderr WARNING on every run for the common,
supported case of a static api_key/auth_token in config, producing
unsuppressable noise with no functional benefit and risking CI stderr
assertions. Restore silent acceptance of static credentials; the
both-set warning and the keycmd backtick/NUL validation are unchanged.
---------
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
* fix(examples): Fix GitLab CI discussions so they highlight all the lines
* fix(examples): Fix GitLab CI suggestions so they show all the lines of the before state
* Fix(examples): Move sha1 computation and add start_line to loop
* fix(exampes): line_range is now only added fro multiline comments
* fix(examples): Added multiline check to suggestions
* fix(examples): resolve GitLab multiline line_code from the MR diff
A multiline discussion's line_code hardcoded old_line=0
(<sha>_0_<new>), which is only valid for purely-added lines; GitLab
rejects it when a boundary falls on an unchanged context line. Resolve
each boundary from the MR diff: added lines keep <sha>_0_<new>, context
lines use their real old line, and line_range is omitted (single-line
anchor) when the diff or a boundary cannot be resolved.
---------
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
