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.
Add .cursor-plugin/plugin.json manifest alongside existing Claude Code
and Codex plugin integrations, reusing the shared SKILL.md. Update all
README versions (EN, zh-CN, ja-JP, ko-KR, ru-RU) with Cursor badge and
installation instructions.
git grep defaults to searching only tracked files, causing code_search
to miss untracked directories like internal/mcp/. Add --untracked flag
in workspace mode so both tracked and untracked files are searched.
Also fix non-git-directory detection to use exit code 128 instead of
hardcoded English error string, making it locale-independent.
* feat(i18n): add localization support for Chinese and update description in package.json
* feat: add internationalization support to various components and views
- Integrated translation functionality using `useT` from `I18nProvider` across multiple components including CustomProviderManager, EnvSetupGuide, FileList, LogViewer, PasswordInput, Select, and various views (CancelledView, ConfigView, DoneView, EmptyView, FailedView, IdleView, RunningView).
- Replaced hardcoded strings with localized strings to enhance user experience for different languages.
- Updated button labels, titles, and hints to reflect the new translation implementation.
* fix: correct regex for validating command names in resolveBin function（预存代码，非本次 PR 引入，阻塞了 lint，这里进行修复）
* chore: update open-code-review-vscode-0.1.0.vsix binary file
* fix: address OpenCodeReview bot findings for i18n PR
- Fix singular '1 hour ago' / '1 小时前' in GitService.formatRelative
- Replace hardcoded 'en' locale with dynamic resolveLocale in CliService.install
- Move hardcoded full-width colon into i18n translation strings
- Narrow locale type from string to SupportedLocale in messages, stores
- Extract toHtmlLang() helper to deduplicate locale→HTML lang mapping
- Replace nested ternary with mapping object in ConfigView
- Add missing trailing newlines to 7 files
- Add jest __mocks__/vscode.js for CliService test
Replace the legacy manual `llm.*` configuration with the modern provider
system across all README localizations (en, zh-CN, ja-JP, ko-KR, ru-RU).
The new structure introduces interactive setup for humans and CLI-based
`ocr config set` commands for CI/CD, covering both built-in and custom
providers.
Slow local models often cannot complete these sub-tasks within the
hardcoded 60-second limit, causing timeouts that the user-provided
--timeout flag cannot override. Raise both to 180s in task_template.json
and scan_template.json to give local models sufficient time.
* fix: ensure current file path is always injected for code_comment and improve line number tracking in resolver
* test: Cover blank-line matching in resolveFromFileContent fallback
Add a regression test for snippets that omit blank lines while the
source file retains them, and document that consecutive matching skips
blank lines on both sides.
* test: Add tests for resolving line numbers with blank lines and CRLF
* feat: add tool_calls field to JSON output for tool usage statistics
Track per-tool invocation counts in llmloop.Runner and expose them
through the ResultProvider interface so both review and scan modes
report tool call statistics in --format json output.
* fix: correct tool_calls counting and ensure stable JSON schema
Move recordToolCall after lookupTool nil check so only actually-executed
tool calls are counted. Always emit tool_calls field in JSON output for
a consistent schema, initializing by_tool to empty map when nil.
* fix: remove omitempty from tool_calls to ensure stable JSON schema
Drop omitempty from the tool_calls struct tag and initialize ToolCalls
in outputJSONNoFiles so the field is always present in JSON output
regardless of execution path.
Commit 18797f8 added the ocr scan feature but only updated the English
README. This syncs all six categories of changes (intro paragraph, quick
start examples, commands table, --exclude flag, ocr scan flags section,
and scan usage examples) to zh-CN, ja-JP, ko-KR, and ru-RU.
* feat: add ocr scan for full-file code review
Introduce a new top-level subcommand `ocr scan` (alias `s`) that reviews
whole files instead of git diffs. Use cases include reviewing unfamiliar
codebases, pre-migration audits, and ad-hoc per-directory reviews.
Architecture splits scan and diff review at the package level so the two
pipelines can evolve independently:
- internal/scan/ new package: file enumeration via `git ls-files`,
full-scan agent, FULL_SCAN_TASK rendering, preview
- internal/llmloop/ new package: shared LLM tool-use loop, three-zone
memory compression, CommentWorkerPool, AgentWarning.
Both internal/agent and internal/scan delegate to
llmloop.Runner; agent and scan never import each other
- internal/agent/ slimmed: LLM loop / compression / token aggregation
moved to llmloop; review-only orchestration remains
- internal/model/ new ScanItem (full-file payload) + Preview /
PreviewEntry / ExcludeReason shared by both modes
- internal/diff/ new gitignore.go exporting helpers reused by scan
- cmd/opencodereview/ new scan_cmd.go; shared.go consolidates startup
(loadCommonContext / loadLLMRuntime), output
(emitRunResult, ResultProvider) and stdout silencing
(quietHandle); review_cmd.go follows the same shape
Template additions:
- FULL_SCAN_TASK: dedicated prompt with Tool-call discipline guidance to
reduce gratuitous tool calls per file
- FULL_SCAN_MAX_TOOL_REQUEST_TIMES (default 60): scan-only per-file budget,
raised over diff's 30 to fit multi-finding files; --max-tools still
composes (only raise, never lower)
In scan mode, file_read_diff is filtered out of MainToolDefs since it has
no useful semantics without a diff.
Tests cover provider enumeration (with temp git repo), template rendering,
filter passes, dependency budget, flag validation, and excludeToolDef.
* feat(scan): v2 — exclude / non-git / split template / plan / batch / dedup / project-summary
Address design-review feedback by evolving `ocr scan` along seven axes
while keeping `ocr review` behavior unchanged:
1. File size cap is now configurable (ScanTemplate.MaxFileSizeBytes,
default 2 MiB; previously a hard-coded 5 MiB). The cap exists only to
bound memory reading; the real review-feasibility gate is the per-file
token budget downstream.
2. Drop the `--all` flag. Bare `ocr scan` now scans the whole repo;
`--path` narrows. Less ceremony, fewer redundant flags.
3. New `--exclude` flag on both review and scan. Comma-separated
gitignore-style patterns; merged with rule.json's exclude layer via
the new shared.applyCLIExcludes helper.
4. Scan supports non-git directories. internal/scan.Provider chooses
between `git ls-files` (full .gitignore semantics) and a
filepath.WalkDir fallback (root .gitignore + ExcludedDirs blocklist)
per isGitRepo probe. loadCommonContext takes a requireGit bool; review
keeps the hard requirement, scan relaxes it.
5. Scan configuration lives in its own file. internal/config/template:
- new ScanTemplate type with LoadScanDefault/ApplyLanguage/Validate
- new embedded scan_template.json
- Template loses the FULL_SCAN_* fields (review template unaffected)
scan.Agent.Args.Template now holds a ScanTemplate; toLoopTemplate
adapts it for llmloop.Runner.
6. New scan phases — each nil-able in the template and toggleable via a
CLI flag, so users can revert to v1 behavior trivially:
* PLAN_TASK (--no-plan): per-file pre-pass that outputs a JSON
summary + checkpoints, embedded into MAIN_TASK as {{plan_guidance}}.
formatPlanGuidance renders to markdown; malformed JSON falls back
to raw text. PLAN_TASK failure never blocks the main loop.
* BATCH_STRATEGY (--batch): files are grouped before dispatch.
"none" preserves v1, "by-language" (default) groups by extension,
"by-directory" groups by first-level subdir. BatchSize caps natural
groups so a single language with 500 files doesn't form one giant
batch. Batches are processed sequentially; files within a batch
remain concurrent up to MaxConcurrency.
* DEDUP_TASK (--no-dedup): per-batch postprocess that asks the LLM
to cluster near-duplicate comments. Output is a `groups` JSON;
every input id must appear exactly once or the result is rejected
and originals are kept (safety: never silently lose comments).
CommentCollector grows Snapshot/Since/ReplaceSince for this.
* PROJECT_SUMMARY_TASK (--no-summary): once-per-run cross-file
summary appended to text output and surfaced as `project_summary`
in JSON output. ResultProvider grows ProjectSummary(); agent.Agent
returns "" (review mode has no project summary).
All four new LLM steps record token usage via runner.RecordUsage so
aggregate counters stay accurate.
7. Tests cover the new pure code paths:
- batch_test.go: 3 strategies, BatchSize cap, language-key edge cases
- dedup_test.go: groups parser, malformed shapes, fence stripping,
payload field selection
- agent_test.go: formatPlanGuidance variants, buildSummaryCommentsList
truncation, maybeRunPlan skip paths
- provider_test.go: non-git directory walker fallback
- template_test.go: ScanTemplate loads / ApplyLanguage / review
template no longer contains scan fields
The seven phases can be reverted independently by toggling flags or
clearing the corresponding optional template fields; nothing forces the
new behavior on existing review users.
* fix(scan): three real bugs surfaced by SCAN_PLAN_TASK self-review
A v2 end-to-end test (ocr scan --path internal/scan/preview.go) had the
PLAN_TASK phase flag three concrete bugs in the scan package itself.
This commit fixes them and adds regression tests.
1. Preview() mutated a.items as a side-effect.
Both Preview and Run wrote to a.items. Calling Preview before Run
silently primed Run with the preview's enumeration instead of
triggering a fresh listFiles. Preview is documented as a read-only
dry-run; uphold that. Local variable now; a.items stays nil after
Preview returns.
2. Preview.result.Entries was nil when there were no items.
With no items at all the loop never ran, so Entries remained nil and
JSON marshalling produced "files":null. Pre-allocate to a non-nil
empty slice so the JSON contract stays "files":[] regardless.
3. Provider.Enumerate and listFilesViaWalk never checked ctx.Done().
On a large repo a cancelled context would still complete the full
walk before the caller saw an error (every iteration costs a stat or
ReadFile syscall). Add the check at the top of each iteration in
both the git-ls-files path and the walker fallback path; the walker
returns ctx.Err() so filepath.WalkDir propagates the cancellation.
Three new regression tests pin the contracts:
- TestPreview_DoesNotMutateAgentItems
- TestPreview_EmptyResultEntriesIsNonNilSlice
- TestProvider_Enumerate_RespectsContextCancellation
* feat(scan): cost estimate + token budget cap; fix file_find on non-git dirs
Two cost-control features and one robustness fix, all surfaced by running
the scanner against a real ~870K-token repository.
Cost estimate (internal/scan/estimate.go):
- Before dispatch, Run prints an order-of-magnitude projection of token
usage (input/output/total), derived from per-file content size × an
assumed round count, plus the optional plan/dedup/summary phases.
- Deliberately reports tokens only, not dollars — pricing varies per
provider/model and a precise figure would mislead. Actual usage is still
reported from the API after the run.
Token budget cap (--max-tokens-budget / ScanTemplate.MaxTokensBudget):
- Caps total token usage for one scan. The gate is checked per file inside
dispatchBatch, right before acquiring a concurrency slot: if tokens
already spent plus a look-ahead estimate of the next file would exceed
the budget, that file and all remaining files are skipped and a
token_budget_reached warning is recorded.
- An earlier batch-level gate was too coarse: with the default by-language
batching, a Go-heavy repo puts most files in one batch, so the gate only
fired between batches and overran the budget ~2.4×. The per-file gate
bounds overrun to roughly one in-flight file per worker (~1.3× at
concurrency=1 in testing).
- 0 = unlimited (unchanged default behavior).
Phase-gate helpers (planEnabled/dedupEnabled/summaryEnabled) consolidate
the "template defines it AND --no-* flag not set" checks so the cost
estimate and the dispatch path agree on which phases will actually run.
file_find non-git fallback (internal/tool/file_find.go):
- `git ls-files` exits 128 in a non-git directory, which spammed failures
when scanning plain directories (scan already supports non-git repos via
the provider's walker, but the file_find tool did not). Now falls back
to filepath.WalkDir honoring the root .gitignore and the default
excluded-dir blocklist when git fails and no specific ref is requested.
Tests:
- estimate_test.go: humanTokens formatting, per-file vs aggregate estimate
consistency, phase scaling, phase-gate tri-state.
- budget_test.go: fake LLM client drives the gate deterministically —
verifies dispatch stops before exceeding budget and that 0 = unlimited.
- file_find_test.go: non-git directory fallback finds files, honors
.gitignore / blocklist, and returns the not-found sentinel correctly.
* docs(readme): document ocr scan subcommand and flags
ocr scan existed but was undiscoverable from the README. Add it to the
intro blurb, Quick Start, the Commands table, Examples, and a dedicated
flags table (path / exclude / preview / max-tokens-budget / no-plan /
no-dedup / no-summary / batch / format / concurrency / rule / repo).
Note non-git support and the pre-run cost estimate. Also backfill the
--exclude flag in the ocr review flags table (added during the v1.3 merge
but never documented).
Flag names and defaults verified against `ocr scan -h`.
* fix(scan): code_search works in non-git directories via git grep --no-index
code_search relied on `git grep`, which exits 128 in a non-git directory —
so `ocr scan` on a plain directory (already supported by file enumeration and
file_find) silently returned errors instead of search results. Detect that
failure and retry with `git grep --no-index --exclude-standard`, which searches
the working tree directly while still honoring .gitignore. Reuses all existing
grep flag/parsing logic; ref-based search still requires a real repo.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(scan): preserve context on compression failure; fix NUL parsing in gitLs
Address three real bugs from the PR #93 automated review that regressed when
compression moved into internal/llmloop:
- Sync compression failure / empty summary now return the original messages
instead of truncating to the frozen zone, which discarded the whole
per-file conversation context.
- Async compression now abandons the job on error instead of applying a
truncated snapshot, and re-applies messages appended while it ran
(snapshotLen), so concurrent tool results are no longer lost.
- scan Provider.gitLs uses cmd.Output() instead of CombinedOutput() so
stderr can't corrupt the NUL-delimited (-z) filename parsing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* feat(llm): update provider model lists
Sync built-in model presets with latest vendor offerings.
* docs: update providers screenshot
Reflect the latest provider list in the config provider TUI.
