Files
历代星辰 e95bdda4f2 feat(cli): add --output flag to write review/scan results to a file (#852)
* feat(cli): add --output flag to write review/scan results to a file

Add `--output <path>` / `-o` to `ocr review` and `ocr scan`, writing the
result JSON or text directly to a UTF-8 file instead of stdout. The file
is created lazily on the first write so a failed run never truncates an
existing target; text-mode files are ANSI-stripped so terminal color
codes never pollute the result file. A `[ocr] Results written to <path>`
hint is printed to stderr once the file is actually created, and failure
output keeps going to stderr so agents always find the failure reason.

Closes #851

Signed-off-by: 历代星辰

* test(cli): cover --output flag parsing and file output behavior

Add tests for --output/-o flag parsing, the stripAnsiWriter state machine
(including escape sequences split across Write calls), lazy file creation
(failed runs leave existing targets untouched, never-written targets are
not created), and the Results-written stderr hint. Adapt existing
emitRunResult / renderComment / outputPreview call sites to the new
io.Writer parameter.

Signed-off-by: 历代星辰

* fix(test): isolate USERPROFILE so Windows tests never touch the real OCR home

os.UserHomeDir() prefers USERPROFILE over HOME on Windows, so
t.Setenv("HOME", ...) alone left tests reading and writing the
developer's real ~/.opencodereview: config tests overwrote config.json
and session/agent tests polluted the sessions store. Add a setTestHome
helper (per affected package) that also overrides USERPROFILE, and route
every scattered HOME override through it.

Signed-off-by: 历代星辰

* fix(cli): propagate output write failures and strip multi-byte ANSI escapes

Address the ocr review findings on the --output feature:
- lazyFileWriter now records the first write error (Err()) and emits the
  "Results written" hint only after a successful write; emitRunResult and
  outputPreview check it after text rendering, so a failed --output write
  (permission, disk full) exits non-zero like JSON mode already does
  instead of silently exiting 0 with no file.
- stripAnsiWriter keeps multi-byte escapes (ESC + intermediate byte,
  DCS/PM/APC strings) inside the escape state so trailing bytes are
  discarded with the sequence instead of leaking into the result file.

Signed-off-by: 历代星辰

* fix(cli): re-parse trailing byte after bare-ESC OSC termination and fix Write return semantics

Addresses review comments on #852:

- stripAnsiWriter ansiOSCEsc: a non-ST byte after ESC is no longer dropped.
  The OSC ends at a bare ESC terminator and the trailing byte is re-parsed —
  an ESC starts a new escape sequence, any other byte is forwarded as text.
  Previously the first byte after a bare-ESC-terminated OSC was silently lost.
- stripAnsiWriter Write: report the underlying dst error but return len(p),
  since the state machine has already consumed the input; returning 0 made a
  caller retrying on n < len(p) feed the same bytes through twice.
- Clarify --output help text: default is stdout and '-' also means stdout.

Adds regression tests for both bugs (bare-ESC + trailing text, bare-ESC + new
escape, and dst-failure return contract).

Signed-off-by: 历代星辰

* fix(cli): reinforce ANSI stripper state machine and validate format flag

* docs(cli): document output flag across localized references and READMEs

* fix(cli): cap escape intermediate bytes and normalize format in output handlers

---------

Signed-off-by: 历代星辰
2026-08-24 15:37:29 +08:00

105 lines
3.3 KiB
Go

// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 alibaba/open-code-review Contributors
package main
import (
"os"
"path/filepath"
"strings"
"testing"
"github.com/alibaba/open-code-review/internal/config/template"
"github.com/alibaba/open-code-review/internal/llm"
)
// loadTestTemplate returns a validated default template for runtime tests.
func loadTestTemplate(t *testing.T) *template.Template {
t.Helper()
tpl, err := template.LoadDefault()
if err != nil {
t.Fatalf("LoadDefault: %v", err)
}
return tpl
}
// TestLoadLLMRuntime_Success resolves an endpoint via OCR_LLM_* env vars (no
// config file on disk, so LoadAppConfig returns nil,nil) and asserts the
// runtime bundle is fully populated.
func TestLoadLLMRuntime_Success(t *testing.T) {
setTestHome(t, t.TempDir())
t.Setenv("OCR_LLM_URL", "https://api.example.test/v1")
t.Setenv("OCR_LLM_TOKEN", "tok-123")
t.Setenv("OCR_LLM_MODEL", "test-model")
tpl := loadTestTemplate(t)
rt, err := loadLLMRuntime(tpl, "", llm.ResolveOptions{})
if err != nil {
t.Fatalf("loadLLMRuntime error: %v", err)
}
if rt.Model != "test-model" {
t.Errorf("model = %q, want test-model", rt.Model)
}
if rt.Client == nil {
t.Error("expected non-nil client")
}
if rt.Collector == nil {
t.Error("expected non-nil collector")
}
if len(rt.MainToolDefs) == 0 {
t.Error("expected main tool defs")
}
if rt.RuntimeConfig.EndpointHost != "api.example.test" {
t.Errorf("endpoint host = %q, want api.example.test", rt.RuntimeConfig.EndpointHost)
}
}
// TestLoadLLMRuntime_BadToolConfig covers the toolsconfig.Load failure branch.
func TestLoadLLMRuntime_BadToolConfig(t *testing.T) {
setTestHome(t, t.TempDir())
tpl := loadTestTemplate(t)
_, err := loadLLMRuntime(tpl, filepath.Join(t.TempDir(), "no-such-tools.json"), llm.ResolveOptions{})
if err == nil || !strings.Contains(err.Error(), "load tools") {
t.Fatalf("err = %v, want load-tools failure", err)
}
}
// TestLoadLLMRuntime_UnresolvableEndpoint covers the ResolveEndpointWithOptions
// failure branch: no config file and no env vars means no endpoint resolves.
func TestLoadLLMRuntime_UnresolvableEndpoint(t *testing.T) {
setTestHome(t, t.TempDir())
// Clear any inherited resolution sources.
t.Setenv("OCR_LLM_URL", "")
t.Setenv("OCR_LLM_TOKEN", "")
t.Setenv("OCR_LLM_MODEL", "")
t.Setenv("ANTHROPIC_BASE_URL", "")
t.Setenv("ANTHROPIC_AUTH_TOKEN", "")
t.Setenv("ANTHROPIC_MODEL", "")
tpl := loadTestTemplate(t)
_, err := loadLLMRuntime(tpl, "", llm.ResolveOptions{})
if err == nil || !strings.Contains(err.Error(), "resolve LLM endpoint") {
t.Fatalf("err = %v, want resolve-endpoint failure", err)
}
}
// TestLoadLLMRuntime_BadAppConfig covers the LoadAppConfig parse-failure branch
// by writing an invalid config.json at the default HOME-based path.
func TestLoadLLMRuntime_BadAppConfig(t *testing.T) {
home := t.TempDir()
setTestHome(t, home)
cfgDir := filepath.Join(home, ".opencodereview")
if err := os.MkdirAll(cfgDir, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.WriteFile(filepath.Join(cfgDir, "config.json"), []byte("{not json"), 0o644); err != nil {
t.Fatalf("write config: %v", err)
}
tpl := loadTestTemplate(t)
_, err := loadLLMRuntime(tpl, "", llm.ResolveOptions{})
if err == nil || !strings.Contains(err.Error(), "load app config") {
t.Fatalf("err = %v, want load-app-config failure", err)
}
}