Skip to content

Commit 9cffb7b

Browse files
wesmclaude
andauthored
TUI markdown rendering, escape sanitization, and test infrastructure (#242)
## Summary - Add glamour-based markdown rendering to TUI review and prompt views with caching, scroll fixes, and configurable tab width - Sanitize non-SGR terminal escape sequences (OSC, DCS, private-mode CSI) in rendered output to prevent terminal injection from untrusted content - Implement CommonMark-compliant fence detection for code block pre-truncation - Move slow CLI tests behind `//go:build integration` tag (unit tests: 35s -> 12s) - Separate Postgres tests into `//go:build postgres` tag to avoid CI failures in jobs without Postgres - Restructure Makefile test targets: `test` (unit), `test-integration` (unit+slow), `test-postgres` (postgres), `test-all` (everything) - Update CI to pass correct build tags per job ## Test plan - [x] Unit tests pass (`go test ./...`) - [x] Integration tests pass (`go test -tags integration ./...`) - [x] Postgres tests pass (`make test-postgres`) - [x] Escape sanitization covers SGR preservation, OSC/DCS/CSI stripping, private-mode CSI, and incomplete sequences - [x] Fence detection handles CommonMark edge cases (indented fences, tilde fences, invalid closers) - [x] Fallback render path applies sanitization 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f237362 commit 9cffb7b

20 files changed

+1397
-497
lines changed

.github/workflows/ci.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ jobs:
2929

3030
- name: Test with race detection (Linux/macOS)
3131
if: matrix.os != 'windows-latest'
32-
run: go test -race ./...
32+
run: go test -race -tags integration ./...
3333

3434
- name: Test (Windows)
3535
if: matrix.os == 'windows-latest'
36-
run: go test ./...
36+
run: go test -tags integration ./...
3737

3838
- name: Build with CGO disabled
3939
run: go build ./...
@@ -52,7 +52,7 @@ jobs:
5252
- name: Test with coverage
5353
# -p 1 serializes package execution to avoid ROBOREV_DATA_DIR race conditions
5454
# between tests in different packages that modify this environment variable
55-
run: go test -race -p 1 -coverprofile=coverage.out ./...
55+
run: go test -race -tags integration -p 1 -coverprofile=coverage.out ./...
5656

5757
- name: Upload coverage
5858
uses: codecov/codecov-action@v4
@@ -84,7 +84,7 @@ jobs:
8484
go-version: '1.24'
8585

8686
- name: Run integration tests
87-
run: go test -tags=integration -v ./internal/storage/... -run Integration
87+
run: go test -tags=postgres -v ./internal/storage/... -run Integration
8888
env:
8989
TEST_POSTGRES_URL: postgres://roborev_test:roborev_test_password@localhost:5433/roborev_test
9090

AGENTS.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ Runtime:
4646

4747
- Tests should be fast and isolated; use `t.TempDir()`.
4848
- Use the `agent = "test"` path to avoid calling real AI agents.
49-
- Suggested commands: `go test ./...`, `go build ./...`, `make install`.
49+
- Slow integration tests use `//go:build integration` and are excluded by default.
50+
- Suggested commands: `go test ./...` (unit), `go test -tags integration ./...` (all), `go build ./...`, `make install`.
5051

5152
## Review/Refine Guidance
5253

CLAUDE.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,19 @@ CLI (roborev) → HTTP API → Daemon (roborev daemon run) → Worker Pool → A
5151
## Commands
5252

5353
```bash
54-
go build ./... # Build
55-
go test ./... # Test
56-
make install # Install to ~/.local/bin
57-
roborev init # Initialize in a repo
58-
roborev status # Check daemon/queue
54+
go build ./... # Build
55+
go test ./... # Test (unit tests only)
56+
go test -tags integration ./... # Test (unit + integration)
57+
make install # Install to ~/.local/bin
58+
roborev init # Initialize in a repo
59+
roborev status # Check daemon/queue
5960
```
6061

62+
Slow integration tests use the `//go:build integration` tag and are excluded
63+
from `go test ./...` by default. They run in CI and can be run locally with
64+
`-tags integration`. Postgres tests use `//go:build postgres` and require a
65+
running Postgres instance (`TEST_POSTGRES_URL` env var).
66+
6167
## Adding a New Agent
6268

6369
1. Create `internal/agent/newagent.go`

Makefile

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
44
LDFLAGS := -X github.com/roborev-dev/roborev/internal/version.Version=$(VERSION)
55

6-
.PHONY: build install clean test test-integration test-all postgres-up postgres-down
6+
.PHONY: build install clean test test-integration test-postgres test-all postgres-up postgres-down test-postgres-ci
77

88
build:
99
@mkdir -p bin
@@ -19,28 +19,32 @@ install:
1919
clean:
2020
rm -rf bin/
2121

22-
# Unit tests only (excludes integration tests)
22+
# Unit tests only (excludes integration and postgres tests)
2323
test:
2424
go test ./...
2525

26-
# Start postgres for integration tests
26+
# Unit + slow integration tests (no postgres required)
27+
test-integration:
28+
go test -tags=integration ./...
29+
30+
# Start postgres for postgres tests
2731
postgres-up:
2832
docker compose -f docker-compose.test.yml up -d --wait
2933

3034
# Stop postgres
3135
postgres-down:
3236
docker compose -f docker-compose.test.yml down
3337

34-
# Integration tests (requires postgres running)
35-
test-integration: postgres-up
38+
# Postgres tests (requires postgres running)
39+
test-postgres: postgres-up
3640
@echo "Waiting for postgres to be ready..."
3741
@sleep 2
3842
TEST_POSTGRES_URL="postgres://roborev_test:roborev_test_password@localhost:5433/roborev_test" \
39-
go test -tags=integration -v ./internal/storage/... -run Integration
43+
go test -tags=postgres -v ./internal/storage/... -run Integration
4044

41-
# Run all tests including integration
42-
test-all: test test-integration
45+
# Run all tests (unit + integration + postgres)
46+
test-all: test-integration test-postgres
4347

44-
# CI target: run integration tests without managing docker (assumes postgres is running)
45-
test-integration-ci:
46-
go test -tags=integration -v ./internal/storage/... -run Integration
48+
# CI target: run postgres tests without managing docker (assumes postgres is running)
49+
test-postgres-ci:
50+
go test -tags=postgres -v ./internal/storage/... -run Integration
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
//go:build integration
2+
3+
package main
4+
5+
import (
6+
"context"
7+
"encoding/json"
8+
"net/http"
9+
"net/http/httptest"
10+
"strings"
11+
"sync/atomic"
12+
"testing"
13+
"time"
14+
15+
"github.com/roborev-dev/roborev/internal/prompt/analyze"
16+
"github.com/roborev-dev/roborev/internal/storage"
17+
)
18+
19+
func TestWaitForAnalysisJob(t *testing.T) {
20+
tests := []struct {
21+
name string
22+
responses []jobResponse // sequence of responses
23+
wantErr bool
24+
wantErrMsg string
25+
}{
26+
{
27+
name: "immediate success",
28+
responses: []jobResponse{
29+
{status: "done", review: "Analysis complete: found 3 issues"},
30+
},
31+
},
32+
{
33+
name: "queued then done",
34+
responses: []jobResponse{
35+
{status: "queued"},
36+
{status: "running"},
37+
{status: "done", review: "All good"},
38+
},
39+
},
40+
{
41+
name: "job failed",
42+
responses: []jobResponse{
43+
{status: "failed", errMsg: "agent crashed"},
44+
},
45+
wantErr: true,
46+
wantErrMsg: "agent crashed",
47+
},
48+
{
49+
name: "job canceled",
50+
responses: []jobResponse{
51+
{status: "canceled"},
52+
},
53+
wantErr: true,
54+
wantErrMsg: "canceled",
55+
},
56+
{
57+
name: "job not found",
58+
responses: []jobResponse{
59+
{notFound: true},
60+
},
61+
wantErr: true,
62+
wantErrMsg: "not found",
63+
},
64+
}
65+
66+
for _, tt := range tests {
67+
t.Run(tt.name, func(t *testing.T) {
68+
var callCount int32
69+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
70+
idx := int(atomic.AddInt32(&callCount, 1)) - 1
71+
if idx >= len(tt.responses) {
72+
idx = len(tt.responses) - 1
73+
}
74+
resp := tt.responses[idx]
75+
76+
switch {
77+
case strings.HasPrefix(r.URL.Path, "/api/jobs"):
78+
if resp.notFound {
79+
json.NewEncoder(w).Encode(map[string]interface{}{"jobs": []interface{}{}})
80+
return
81+
}
82+
job := storage.ReviewJob{
83+
ID: 42,
84+
Status: storage.JobStatus(resp.status),
85+
Error: resp.errMsg,
86+
}
87+
json.NewEncoder(w).Encode(map[string]interface{}{"jobs": []storage.ReviewJob{job}})
88+
89+
case strings.HasPrefix(r.URL.Path, "/api/review"):
90+
json.NewEncoder(w).Encode(storage.Review{
91+
JobID: 42,
92+
Output: resp.review,
93+
})
94+
}
95+
}))
96+
defer ts.Close()
97+
98+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
99+
defer cancel()
100+
101+
review, err := waitForAnalysisJob(ctx, ts.URL, 42)
102+
103+
if tt.wantErr {
104+
if err == nil {
105+
t.Fatal("expected error, got nil")
106+
}
107+
if !strings.Contains(err.Error(), tt.wantErrMsg) {
108+
t.Errorf("error %q should contain %q", err.Error(), tt.wantErrMsg)
109+
}
110+
return
111+
}
112+
113+
if err != nil {
114+
t.Fatalf("unexpected error: %v", err)
115+
}
116+
if review == nil {
117+
t.Fatal("expected review, got nil")
118+
}
119+
if review.Output != tt.responses[len(tt.responses)-1].review {
120+
t.Errorf("got review %q, want %q", review.Output, tt.responses[len(tt.responses)-1].review)
121+
}
122+
})
123+
}
124+
}
125+
126+
func TestRunAnalyzeAndFix_Integration(t *testing.T) {
127+
// This tests the full workflow with mocked daemon and test agent
128+
tmpDir := createTestRepo(t, map[string]string{
129+
"main.go": "package main\n",
130+
})
131+
132+
ts, state := newMockServer(t, MockServerOpts{
133+
JobIDStart: 99,
134+
ReviewOutput: "## CODE SMELLS\n- Found duplicated code in main.go",
135+
DoneAfterPolls: 2,
136+
})
137+
138+
cmd, output := newTestCmd(t)
139+
140+
analysisType := analyze.GetType("refactor")
141+
opts := analyzeOptions{
142+
agentName: "test",
143+
fix: true,
144+
fixAgent: "test",
145+
reasoning: "fast",
146+
}
147+
148+
err := runAnalyzeAndFix(cmd, ts.URL, tmpDir, 99, analysisType, opts)
149+
if err != nil {
150+
t.Fatalf("runAnalyzeAndFix failed: %v", err)
151+
}
152+
153+
// Verify the workflow was executed
154+
if atomic.LoadInt32(&state.JobsCount) < 2 {
155+
t.Error("should have polled for job status")
156+
}
157+
if atomic.LoadInt32(&state.ReviewCount) == 0 {
158+
t.Error("should have fetched the review")
159+
}
160+
if atomic.LoadInt32(&state.AddressCount) == 0 {
161+
t.Error("should have marked job as addressed")
162+
}
163+
164+
// Verify output contains analysis result
165+
outputStr := output.String()
166+
if !strings.Contains(outputStr, "CODE SMELLS") {
167+
t.Error("output should contain analysis result")
168+
}
169+
if !strings.Contains(outputStr, "marked as addressed") {
170+
t.Error("output should confirm job was addressed")
171+
}
172+
}

0 commit comments

Comments
 (0)