83441bfbac
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 12s
CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 37s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m0s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m12s
Add persona system for specialized review roles. Each persona defines: - A specific review focus (security, architecture, documentation) - Custom system prompt additions - Personality/tone adjustments Built-in personas: security, architect, docs Custom personas: load from JSON via persona-file flag Includes workspace validation to prevent path traversal attacks. Closes #51
212 lines
4.7 KiB
Go
212 lines
4.7 KiB
Go
package review
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
)
|
|
|
|
func TestLoadBuiltinPersona(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
personaName string
|
|
wantErr bool
|
|
wantDisplay string
|
|
}{
|
|
{
|
|
name: "security persona",
|
|
personaName: "security",
|
|
wantErr: false,
|
|
wantDisplay: "Security Specialist",
|
|
},
|
|
{
|
|
name: "architect persona",
|
|
personaName: "architect",
|
|
wantErr: false,
|
|
wantDisplay: "Architecture Reviewer",
|
|
},
|
|
{
|
|
name: "docs persona",
|
|
personaName: "docs",
|
|
wantErr: false,
|
|
wantDisplay: "Documentation Reviewer",
|
|
},
|
|
{
|
|
name: "unknown persona",
|
|
personaName: "nonexistent",
|
|
wantErr: true,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
p, err := LoadBuiltinPersona(tt.personaName)
|
|
if tt.wantErr {
|
|
if err == nil {
|
|
t.Error("expected error, got nil")
|
|
}
|
|
return
|
|
}
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
if p.Name != tt.personaName {
|
|
t.Errorf("Name = %q, want %q", p.Name, tt.personaName)
|
|
}
|
|
if p.DisplayName != tt.wantDisplay {
|
|
t.Errorf("DisplayName = %q, want %q", p.DisplayName, tt.wantDisplay)
|
|
}
|
|
if p.Identity == "" {
|
|
t.Error("Identity should not be empty")
|
|
}
|
|
if len(p.Focus) == 0 {
|
|
t.Error("Focus should not be empty")
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestListBuiltinPersonas(t *testing.T) {
|
|
names := ListBuiltinPersonas()
|
|
if len(names) == 0 {
|
|
t.Fatal("expected at least one built-in persona")
|
|
}
|
|
|
|
// Check for expected personas
|
|
expected := map[string]bool{"security": false, "architect": false, "docs": false}
|
|
for _, name := range names {
|
|
if _, ok := expected[name]; ok {
|
|
expected[name] = true
|
|
}
|
|
}
|
|
for name, found := range expected {
|
|
if !found {
|
|
t.Errorf("expected built-in persona %q not found", name)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestLoadPersonaFromFile(t *testing.T) {
|
|
// Create a temp persona file
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "test.json")
|
|
|
|
content := `{
|
|
"name": "test",
|
|
"display_name": "Test Persona",
|
|
"identity": "You are a test persona.",
|
|
"focus": ["testing"],
|
|
"ignore": ["nothing"],
|
|
"severity": {
|
|
"major": "Big problems",
|
|
"minor": "Small problems",
|
|
"nit": "Tiny problems"
|
|
}
|
|
}`
|
|
|
|
if err := os.WriteFile(path, []byte(content), 0644); err != nil {
|
|
t.Fatalf("failed to write test file: %v", err)
|
|
}
|
|
|
|
p, err := LoadPersona(path)
|
|
if err != nil {
|
|
t.Fatalf("LoadPersona failed: %v", err)
|
|
}
|
|
|
|
if p.Name != "test" {
|
|
t.Errorf("Name = %q, want %q", p.Name, "test")
|
|
}
|
|
if p.DisplayName != "Test Persona" {
|
|
t.Errorf("DisplayName = %q, want %q", p.DisplayName, "Test Persona")
|
|
}
|
|
}
|
|
|
|
func TestLoadPersonaValidation(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
json string
|
|
wantErr string
|
|
}{
|
|
{
|
|
name: "missing name",
|
|
json: `{"identity": "test"}`,
|
|
wantErr: "name is required",
|
|
},
|
|
{
|
|
name: "missing identity",
|
|
json: `{"name": "test"}`,
|
|
wantErr: "identity is required",
|
|
},
|
|
{
|
|
name: "display_name defaults to name",
|
|
json: `{"name": "test", "identity": "test identity"}`,
|
|
// No error expected - should succeed
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "test.json")
|
|
if err := os.WriteFile(path, []byte(tt.json), 0644); err != nil {
|
|
t.Fatalf("failed to write test file: %v", err)
|
|
}
|
|
|
|
p, err := LoadPersona(path)
|
|
if tt.wantErr != "" {
|
|
if err == nil {
|
|
t.Errorf("expected error containing %q, got nil", tt.wantErr)
|
|
return
|
|
}
|
|
if !contains(err.Error(), tt.wantErr) {
|
|
t.Errorf("error = %q, want containing %q", err.Error(), tt.wantErr)
|
|
}
|
|
return
|
|
}
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
// Check display_name defaulting
|
|
if p.DisplayName == "" {
|
|
t.Error("DisplayName should default to Name")
|
|
}
|
|
if p.DisplayName != p.Name {
|
|
t.Errorf("DisplayName should default to Name, got %q", p.DisplayName)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestLoadPersonaFileNotFound(t *testing.T) {
|
|
_, err := LoadPersona("/nonexistent/path/persona.json")
|
|
if err == nil {
|
|
t.Error("expected error for nonexistent file")
|
|
}
|
|
}
|
|
|
|
func TestLoadPersonaInvalidJSON(t *testing.T) {
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "invalid.json")
|
|
if err := os.WriteFile(path, []byte("not json"), 0644); err != nil {
|
|
t.Fatalf("failed to write test file: %v", err)
|
|
}
|
|
|
|
_, err := LoadPersona(path)
|
|
if err == nil {
|
|
t.Error("expected error for invalid JSON")
|
|
}
|
|
}
|
|
|
|
func contains(s, substr string) bool {
|
|
return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr))
|
|
}
|
|
|
|
func containsHelper(s, substr string) bool {
|
|
for i := 0; i <= len(s)-len(substr); i++ {
|
|
if s[i:i+len(substr)] == substr {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|