Commit graph

124 commits

Author SHA1 Message Date
oo0-0-0oo
8ef497c619
feat: add Python code review rules (#343)
* feat: add Python code review rules

Add python.md rule doc for reviewing .py files and wire "**/*.py" into
the system rule map so Python files no longer fall back to default.md.
Rules follow the precision-first house style: security/correctness are
blocking, style is not, and noise-prone sections carry explicit
"do not report" guards.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: align identity-comparison rule with precision-first principle

Split the identity/equality section so severity is explicit: `is`
against literals and `== True/False` are real correctness risks, while
`== None` vs `is None` is a style preference reported as minor. This
removes the section's conflict with the doc's own "style is non-blocking"
header.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-07-09 20:04:18 +08:00
Nikunj Tyagi
bf0af144fd
fix(viewer): fix monospace font rendering in pre/code blocks on Windows (#328) 2026-07-09 16:09:36 +08:00
Ayrton
44ef6d7fc7
feat: enhance token usage resolution for OpenAI and Anthropic compati… (#223)
* feat: enhance token usage resolution for OpenAI and Anthropic compatibility

Recognize OpenAI cached_tokens and wrapped proxy response paths, add tests
for path priority and provider-specific total token fallback semantics.

Co-authored-by: Cursor <cursoragent@cursor.com>

* style: format

* chore: Add tests and improve comments

* fix: Remove provider specific protocol

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-07-09 14:35:47 +08:00
munsunouk
efe2b2e50b
fix(tool): reject traversal pathspecs in code_search (#303) 2026-07-09 13:45:24 +08:00
MuoDoo
f35437671c
feat(review): add resumable sessions and session inspection (#306)
* feat(review): add session resume support

* Add session inspection command

* fix(review): correct session resume checkpoint handling

* fix(review): require completed main loop for resume checkpoints

* docs: document review resume sessions

* docs: refine resume session guidance
2026-07-09 11:43:11 +08:00
kite
e2d75b732a
test: fix golangci-lint errcheck/staticcheck issues in test code (#323)
Some checks are pending
CI / test (push) Waiting to run
2026-07-08 22:46:18 +08:00
xujiejie
46dde274d8
Feat/telemetry http exporter (#314)
* feat(telemetry): add OTLP HTTP exporter and print TraceID

- Add HTTP/protobuf exporter support alongside existing gRPC exporter
- Route based on OTEL_EXPORTER_OTLP_PROTOCOL config (http/protobuf vs grpc)
- Print TraceID to stderr when telemetry is enabled for easier correlation
- Add corresponding unit tests

* feat(telemetry): add span coverage for LLM calls, tool execution, plan/filter phases

- Add StartLLMSpan / RecordLLMResult helpers (span.go), symmetric with
  existing StartToolSpan / RecordToolResult
- Wrap LLM completion calls in llmloop.RunPerFile with llm.request spans
- Wrap all three tool execution paths in executeToolCall with
  tool.execute.* spans (dynamic tools, code_comment sync/async, other tools)
- Add plan.execute span around executePlanPhase
- Add main.loop span around RunPerFile call in executeSubtask
- Add review_filter.execute span around executeReviewFilter, with
  comments.before / comments.filtered attributes
- Record llm.error attribute on LLM failures for diagnosability
- Record review.repo / review.from / review.to / review.model on the
  top-level review.run span
- Metrics (RecordLLMRequest / RecordToolCall) are preserved alongside
  the new spans — they serve different purposes (aggregate dashboards
  vs per-run diagnosis)

Verified end-to-end against Sunfire (OTLP HTTP gateway): full span tree
observed for review.run -> subtask.execute -> plan.execute/main.loop/
review_filter.execute -> llm.request/tool.execute.*

* fix(telemetry): address CR findings — span error handling, async span lifecycle, protocol robustness

- Add span.RecordError(err) to RecordLLMResult and RecordToolResult for
  consistency with EndSpan
- Use OTel standard pattern (span.SetStatus + span.RecordError) in error
  paths of review.run, plan.execute, main.loop, review_filter.execute
- Move async code_comment span end into pool.Submit callback so span
  duration reflects actual execution time
- Unify time.Since(startTime) in code_comment error path to a single dur
- Remove http/json from supported OTLP protocols (not actually implemented)
- Add stderr warning when unknown OTLP protocol falls back to gRPC

* feat(telemetry): include trace_id in JSON output, restrict stderr to text format

- Add trace_id as top-level field in jsonOutput struct (omitempty)
- JSON format: trace_id in structured response for programmatic extraction
- Text format: TraceID printed to stderr for human debugging
- Telemetry disabled: trace_id field omitted entirely

* fix: address PR review findings

- loop.go: wrap async span lifecycle in defer to prevent leak on panic
- exporter.go: update parseOTLPEndpoint comment to reflect gRPC+HTTP usage
- scan_cmd.go: align traceID extraction and OTel error handling with review_cmd
- output.go/shared.go: propagate traceID to outputJSONNoFiles for consistency
- agent.go: move comments.filtered attribute before early return so 0 is
  distinguishable from not-executed

* feat(telemetry): address PR review — http/json routing, LLM span coverage, trace_id tests

- Route http/json to HTTP exporter (Go OTel SDK HTTP transport only
  supports protobuf serialization; users need HTTP transport, not JSON encoding)
- Add llm.request spans to executePlanPhase, executeReviewFilter, and
  ReLocateComment with Usage nil-safety consistent with loop.go
- Add trace_id assertions to output helper tests and emitRunResult
  end-to-end tests using real TracerProvider

* docs: add OTLP protocol selection and endpoint format to telemetry section

Sync across all 5 README language versions (en, zh-CN, ja-JP, ko-KR, ru-RU).

* fix: unify time.Since in async code_comment defer to single dur variable
2026-07-08 13:12:21 +08:00
chethanuk
14f1c22a60
feat(viewer): dark mode + system monospace font (#304) (#312)
Enhance the embedded viewer stylesheet per #304:

- Add automatic dark mode via `prefers-color-scheme` (no JS/toggle).
  The full colour palette — surfaces, text, borders, accents, task/badge
  brand colours — lives in `:root` custom properties, with a single dark
  block overriding the values. Light-mode colours are unchanged.
- Declare `color-scheme: light dark` so native scrollbars and form
  controls follow the active theme.
- Use the system monospace stack (`ui-monospace, ...`) via a `--mono`
  variable, replacing the hardcoded font stacks.
- Dark tokens chosen to clear WCAG AA (>=4.5:1) for small text, and dim
  brand colours (task-main, task-default) lifted for the dark surface.

Verified by rendering all three viewer pages in light and dark with a
headless browser: computed colours, contrast ratios, resolved theme
variables, and `color-scheme` asserted programmatically.
2026-07-08 13:06:02 +08:00
chethanuk
c973e581ec
fix(tool): resolve file_read paths against git top-level in monorepos (#309)
Some checks are pending
CI / test (push) Waiting to run
* fix(tool): resolve file_read paths against git top-level in monorepos

ocr review from a monorepo subdirectory failed with "file not found" (#287):
git reports diff and `git show HEAD:<path>` paths relative to the repo root,
but RepoDir was scoped to the invocation subdirectory, producing a double
prefix. resolveWorkingDir now anchors RepoDir at `git rev-parse
--show-toplevel` on the review path (requireGit=true); scan keeps the CWD so
its `git ls-files` walk stays scoped.

The top-level lookup uses a stdout-only git helper so stderr notices can't
pollute the path, and fails loudly if --show-toplevel errors or is empty
(e.g. a bare repo) instead of silently reusing the subdirectory. Adds
regression tests for the subdir hoist, the scan-path scoping, git-show
resolution of root-relative paths, and the bare-repo failure.

* docs(rules): document repo-root rule.json resolution in monorepos

Since #287 anchored RepoDir at the git top-level, ocr review from a
monorepo subdirectory loads the repo-root .opencodereview/rule.json
rather than a subdir-local one. Call out this user-visible behavior at
loadProjectRule so the scope change isn't a surprise (review feedback).
2026-07-07 20:06:28 +08:00
kite
a2e08b77a1
feat(review): add structured category and severity to findings (#311)
Some checks are pending
CI / test (push) Waiting to run
Add two structured fields, category and severity, to every review finding
so CI integrations can sort, group, filter, or gate builds without
re-parsing natural-language comment text.

- Tool schema (tools.json): add category/severity as enum-constrained,
  required properties of code_comment. severity is limited to
  critical/high/medium/low (info dropped, since LLMs struggle to
  distinguish low from info).
- System prompt (task_template.json) is intentionally left untouched to
  avoid the review-quality regression observed on the benchmark suite;
  the tool schema alone drives field population.
- JSON output: category/severity are flat siblings of content/start_line,
  omitted entirely when empty (backward compatible).
- CLI output: render an inline [category - severity] badge before the
  comment, colored by severity.
- Sync docs across all five README locales.
2026-07-07 13:08:41 +08:00
wxwxwxw_orange
39eb0f3984
fix(tui): persist official-tab models and refine saved secret hint (#260)
Some checks are pending
CI / test (push) Waiting to run
* fix(tui): persist official-tab models and refine saved secret hint

Persist user-added models to providers.<name>.models on the official tab.
When an API key or auth token is already saved, show a replace hint with a
prefix/suffix fingerprint (skipped for short keys), use a fixed mask placeholder,
and ensure typing or paste replaces the saved value instead of re-saving it.

* fix(tui): model add/delete UX and config wizard hardening
- Add model add/delete in config provider and config model (official + custom)
- Show d Delete only on model rows; green highlight when selected
- Improve Esc cancel text; track savedInSession to avoid misleading messages
- Reload config on save failure; read registry models fresh after reload
- Export llm.ModelListContains; fix config model persist using registry-only check

* fix(tui): defer provider config until confirm and harden API key UX
Only persist provider/model on wizard confirm; keep in-session picks via
sessionModelPick. Validate API key before quit, clear saved keys when emptied,
and improve official env-var hints and custom edit clear behavior.

* feat(tui): show active model suffix on official provider list
Align Official tab with Custom: display (model) next to the active preset
when cfg.Provider matches and a global model is configured.
2026-07-06 20:38:39 +08:00
munsunouk
b82fcc31f2
feat: add Astro-specific review rules (#289) 2026-07-03 20:05:34 +08:00
munsunouk
8c9778860d
fix: support Astro files in review filters (#286) 2026-07-03 15:47:14 +08:00
ScarletCarpet
64398465d4
feat(viewer): make human readable token usage for session (#278)
* < 1,000: show the raw number (e.g. 842)
* >= 1,000: show as K (e.g. 1.22K)
* >= 1,000,000: show as M (e.g. 1.21M)

hover shows exact number.
2026-07-03 12:01:46 +08:00
paker
589a7249f5
fix(telemetry): support http:// scheme in otlp_endpoint for insecure gRPC (#280)
* fix(telemetry): support http:// scheme in otlp_endpoint for insecure gRPC

Parse the otlp_endpoint scheme before creating the OTLP gRPC exporters:
- http://host:port  -> strip scheme, call WithInsecure() (plaintext gRPC)
- https://host:port -> strip scheme, keep default TLS
- host:port          -> unchanged, keep default TLS (backward compatible)

Scheme matching is case-insensitive. Applies to both the trace and
metric exporters in initOTLPProviders.

Fixes #268

* fix(telemetry): trim trailing slash from otlp_endpoint after scheme strip

Addresses review feedback: a URL-style endpoint with a trailing slash
(e.g. "http://localhost:4317/") left the trailing "/" in the address
passed to WithEndpoint(), which expects a bare host:port with no path
and could cause connection failures.
2026-07-03 11:26:03 +08:00
Mountain Ghost. W
74a2ac1c1c
feat(llm): add z-ai-coding provider for GLM Coding Plan endpoint (#258)
Some checks are pending
CI / test (push) Waiting to run
Deploy Pages / build (push) Waiting to run
Deploy Pages / deploy (push) Blocked by required conditions
* feat(llm): add z-ai-coding provider for GLM Coding Plan endpoint

Z.AI (智谱) subscribers to the GLM Coding Plan must route requests through
the dedicated coding endpoint (https://open.bigmodel.cn/api/coding/paas/v4)
for them to be billed against the subscription quota. The existing z-ai
provider points at the generic pay-as-you-go endpoint
(https://open.bigmodel.cn/api/paas/v4), so Coding Plan keys silently drain
the wallet balance instead of consuming the plan quota, surfacing as a
spurious "1113 余额不足" error even when the plan is barely used.

Add a dedicated z-ai-coding provider following the existing *-tokenplan
pattern (dashscope/dashscope-tokenplan, tencent-tokenhub/hy-tokenplan).
It reuses Z_AI_API_KEY — the same key authenticates against both endpoints,
so selecting this provider is all that's needed to activate the plan. The
model list is restricted to the models officially supported by the Coding
Plan to avoid selecting a non-plan model that falls back to wallet billing.

- internal/llm/providers.go: register z-ai-coding preset
- extensions/vscode/src/shared/providers.ts: mirror the preset (kept in
  sync with the Go registry per the file header)
- internal/llm/providers_test.go: update the sorted provider list assertion

Co-Authored-By: Oz <oz-agent@warp.dev>

* docs(pages): add Z.AI GLM Coding Plan config tip to docs page

Subscribers to the Z.AI (Zhipu) GLM Coding Plan must route requests through
the dedicated coding endpoint (https://open.bigmodel.cn/api/coding/paas/v4)
to bill against the plan quota. The default z-ai preset points at the generic
pay-as-you-go endpoint, so coding-plan keys silently drain the wallet and
surface a spurious "1113 余额不足" error — a recurring trap for new users.

Add a provider-specific callout at the end of the Docs config section showing
the one-line fix that works today on any released version:

  ocr config set providers.z-ai.url https://open.bigmodel.cn/api/coding/paas/v4

This complements the z-ai-coding provider added in the previous commit: the
provider gives a native first-class option going forward, while this doc tip
rescues users already running released builds. Copy/localized for zh/en/ja.

Co-Authored-By: Oz <oz-agent@warp.dev>

* fix(llm): address review feedback for z-ai-coding provider

- Switch z-ai-coding to a dedicated Z_AI_CODING_API_KEY env var instead
  of reusing Z_AI_API_KEY, matching the tokenplan-provider convention so
  pay-as-you-go and Coding Plan keys can be configured independently
- Remove comment blocks from the z-ai-coding presets (Go and TS) to keep
  the registry as plain data consistent with the other entries
- Revert pages/ changes (i18n + DocsPage.tsx); provider docs are out of
  scope for a provider-registration PR

Co-Authored-By: Oz <oz-agent@warp.dev>

---------

Co-authored-by: mountainwu <mountainwu@kuainiugroup.com>
Co-authored-by: Oz <oz-agent@warp.dev>
2026-07-02 19:23:17 +08:00
Eldar Shlomi
fef4314d46
fix(agent): recover from panics in per-file review and comment-pool goroutines (#171) (#182)
Some checks are pending
CI / test (push) Waiting to run
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).
2026-07-01 23:23:12 +08:00
kite
6adcb1ecd4
feat: add standard MCP tool support (#212)
* 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
2026-07-01 19:10:16 +08:00
kite
52a51f86f8 test(diff): add tests for gitignore pattern matching and mode getters 2026-07-01 14:27:07 +08:00
kite
d84d76b6dc fix(session): isolate test sessions to test-sessions subdirectory
Some checks are pending
CI / test (push) Waiting to run
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.
2026-06-30 13:23:21 +08:00
kite
c076b8e17f docs(llm): clarify timeout override semantics and resolution-path coverage 2026-06-30 11:25:21 +08:00
MurphyYi
eae863502b
fix: add configurable HTTP timeout for LLM API calls (#238)
* 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
2026-06-30 11:22:24 +08:00
kite
f2a6f7c5c7 ci: add 80% statement coverage threshold check
Add coverage threshold enforcement in CI workflow and Makefile to satisfy
OpenSSF Best Practices Silver badge test_statement_coverage80 criterion.
2026-06-27 11:13:22 +08:00
zephyrq-z
b109baf0c2
Feature/rule link to files (#226)
* 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)
2026-06-27 11:06:38 +08:00
kite
30c083c0de test: improve coverage for tool package from 79.5% to 93.6%
Add tests for response_message, stub providers, code_search Execute/Tool,
filereader commit mode with and without Runner, file_find git repo modes,
file_read error paths, and comment_collector ReplaceSince edge cases.
2026-06-27 11:00:23 +08:00
kite
793bbc6a61 test: improve coverage for telemetry package and exclude extensions from build targets
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.
2026-06-27 10:43:45 +08:00
kite
38fc691498 fix: skip permission-based tests when running as root in CI 2026-06-27 00:42:41 +08:00
kite
be4b0b0aa9 test: improve coverage for viewer package from 35% to 92%
Add comprehensive tests for handler, server, and store modules including
LoadSession full parsing, template rendering, and error/edge-case paths.
2026-06-27 00:34:12 +08:00
kite
9f71753f68 test: improve coverage for template and pathutil packages
template: 71.2% → 84.9% — add Validate error-branch tests for both
Template and ScanTemplate, ApplyLanguage coverage for optional scan
tasks (DedupTask, ProjectSummaryTask), nil-optional-task path, and
non-system message skip verification.

pathutil: add tests for non-existent path error, relative path
resolution, nested symlink traversal, and additional WithinBase
edge cases.
2026-06-27 00:22:30 +08:00
kite
453c4f9c76 test: add coverage tests for agent, llm, llmloop, and scan packages
Raise statement coverage to 80%+ across four core packages to meet
FLOSS best practice badge criteria. Key additions:

- internal/scan (67% → 90%): getters, lookupDiff, filterScanItems,
  whyExcluded all branches, extFromPath, maybeRunPlan/ProjectSummary/
  Dedup success paths, executeSubtask, Run pipeline, dispatchSubtasks.
- internal/agent (60% → 83%): getters, filterDiffs, findDiff,
  resolveSystemRule, injectDiffMap, executeReviewFilter, executePlanPhase,
  executeSubtask, dispatchSubtasks.
- internal/llmloop (59% → 80%): warnings, tool calls, usage recording,
  compression lifecycle (cancel/tryApply/run/trigger), partitionMessages.
- internal/llm (67% → 80%): parseBpeData, embedded BPE loader, message
  constructors, ExtractText, ChatResponse helpers, parseShellRC.
2026-06-27 00:15:28 +08:00
kite
110e5284fb fix: stabilize TestDiscoverRepos_FindsRepos with explicit mtime ordering
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.
2026-06-26 23:31:51 +08:00
kite
b3eb4b3491 test: add unit tests for output_helpers, config, model, stdout, and telemetry packages 2026-06-26 23:26:49 +08:00
kite
904ecd3ae1 test: expand unit test coverage for agent, llm, llmloop, and tool packages
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.
2026-06-26 23:09:11 +08:00
kite
98fe309f03 test: add unit tests for pure logic functions across 6 packages
Add table-driven tests for pure computation functions with no external
dependencies, improving overall coverage from 45.9% to 48.8%.

Covered functions:
- suggestdiff: ComputeLineDiff (LCS algorithm)
- tool: CommentCollector, DiffMap, FileReadDiff, Registry, ParseReviewMode, scanLines
- llmloop: CountMessagesTokens, groupIntoRounds, partitionMessages, StripMarkdownFences
- agent: buildFilterCommentsJSON, parseFilterResponse, extFromPath, formatToolDefs, BuildToolDefs
- viewer: truncateText, formatDuration, formatTime
2026-06-26 22:41:35 +08:00
kite
e1a6a404ba feat(ci): add Sigstore attestation for release artifacts
Add build provenance attestation to the release workflow using
actions/attest-build-provenance with OIDC keyless signing.
Document release signature verification in SECURITY.md.
2026-06-26 22:18:03 +08:00
Hyeseong Kim
a1e4cf6b43
feat: support HTTP extra headers (#169)
* feat: support HTTP extra headers

Resolves #166

* update documentation; assisted-by GLM-5.1

* Update cmd/opencodereview/config_cmd.go

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update internal/llm/resolver.go

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* prevent header conflicts

* allow quoted values

* update docs

* add user-agent to extra_headers blocklist

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-06-26 15:42:29 +08:00
kite
779c4a8290 fix: enable code_search to find content in untracked files
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.
2026-06-26 00:01:44 +08:00
kite
034d512b15
fix: increase REVIEW_FILTER_TASK and RE_LOCATION_TASK timeout from 60s to 180s (#208) (#218)
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.
2026-06-25 20:07:07 +08:00
Ayrton
344f981156
fix: ensure current file path is always injected for code_comment and… (#193)
* 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
2026-06-25 16:29:09 +08:00
kite
63b0f02d27
feat: add tool_calls field to JSON output (#216)
* 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.
2026-06-25 13:53:06 +08:00
css521
18797f8c05
feat: add ocr scan for full-file code review (#93)
* 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>
2026-06-24 22:07:25 +08:00
kite
4a2370f9cd fix: make option-like ref test locale-independent
Replace English git error message matching with a locale-agnostic
"Error:" prefix check to fix test failures on non-English systems.
2026-06-24 21:19:06 +08:00
wxwxwxw_orange
9d129e2c10
feat(llm): update provider model lists (#202)
* 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.
2026-06-24 19:55:23 +08:00
kite
71b3353153 fix: skip empty rule entries in resolver to restore fallback behavior
PR #161 refactored matchProjectRule into matchProjectRuleEntry but
removed the empty-rule guard, causing entries with rule:"" to match
and return an empty string instead of falling through to the next
layer or system rule. This restores the skip-on-empty semantics while
honoring merge_system_rule:true with empty rule as "lock system rule".
2026-06-24 18:59:18 +08:00
不许对我狗叫丶
8196b07b4c
Add option to merge system and user review rules (#161)
* feat: add a command param to choose whether append user rule after system rule

* feat: add some unit tests

* fix: remove unused merge system rule agent arg

* feat: Adjust the rule concatenation logic

* fix: fix typo

* test: make merge system rule tests behavior-focused

* test: update the param usage

* test: add param introduction

* test: remove unnecessary blank lines

* feat: support merged rule details in rules check

* feat: add some unit tests

* feat: support per-rule system rule merging

* fix: consider corner case

* fix: preserve first matching rule entry

* Adjust project rule detail argument order
2026-06-24 17:47:03 +08:00
不许对我狗叫丶
da723a6edc
chore: remove deprecated MiMo v2 models (#192) 2026-06-23 20:54:55 +08:00
zephyrq-z
ab694a38ed
feat: add Baidu Qianfan provider and update provider list (#188) 2026-06-23 15:22:12 +08:00
kite
dc93166205 feat(rules): add dedicated review rules for GitHub Actions and GitHub config YAML
The generic YAML rule only checked spelling in keys, which is insufficient
for CI/CD workflow files. Add layered rules with security, correctness,
reliability checks for .github/workflows/ and structure validation for
other .github/ config files (issue templates, release config).
2026-06-22 17:59:16 +08:00
kite
2f3ed41009 refactor(agent): split agent.go into util.go and compression.go, fix compression bugs
Extract utility functions (stripEmptyPlanBlock, stripMarkdownFences,
buildMessageXML, copyMessages, countMessagesTokens, reviewModeString,
detectGitBranch) into util.go and all compression logic into compression.go
to improve readability (agent.go: 1489 → 1114 lines).

Fix pre-existing compression bugs from commit 14adf6f:
- Return unmodified messages on compression failure instead of truncating
  to frozen zone (which discarded all conversation context)
- Check error in async compression goroutine to avoid applying failed results
- Track snapshot length to append post-snapshot messages when applying async
  compression, preventing silent message loss
- Strip old <previous_review_summary> before appending new one to prevent
  cumulative frozen zone bloat across multiple compressions
- Guard pendingJob cleanup to only clear when it matches the completed job,
  preventing accidental clearing of a newly scheduled job
- Add context.Context and 5s timeout to detectGitBranch
- Deep copy ToolCalls in copyMessages to match session/history.go
- Reuse finalCount in addNextMessage to avoid redundant token counting
2026-06-21 21:20:40 +08:00
kite
b74e5bd53f refactor(template): extract prompt strings into standalone .md files
Move all LLM prompt content from inline JSON strings in task_template.json
to individual Markdown files under prompts/, following the same embed.FS
pattern used by internal/config/rules/. This improves maintainability by
enabling native syntax highlighting, cleaner diffs, and separation of
prompt content from runtime configuration.

Also removes two dead config fields (TOOL_REQUEST_WAIT_TIME_MS,
MAX_SUBTASK_EXECUTION_TIME_MINUTES) that had no runtime consumers.
2026-06-21 21:20:40 +08:00