diff --git a/README.md b/README.md index 5bf937a..bae0306 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,6 @@ 자연어 기반 코딩 컨벤션 관리 및 검증 도구 -[![Test Coverage](https://img.shields.io/badge/coverage-view%20report-blue)](https://devsymphony.github.io/sym-cli/coverage.html) - ## 빠른 시작 ### 1. 설치 @@ -15,15 +13,10 @@ npm i -g @dev-symphony/sym ### 2. 초기화 ```bash -# GitHub OAuth 로그인 -sym login - # 프로젝트 초기화 (.sym/ 폴더 생성, MCP 자동 설정) sym init ``` -> **참고**: LLM 기반 컨벤션 변환을 사용하려면 `OPENAI_API_KEY` 환경변수가 필요합니다. - ### 3. 사용 **웹 대시보드로 컨벤션 편집:** @@ -61,7 +54,6 @@ sym dashboard | 명령어 | 설명 | |--------|------| -| `sym login` | GitHub OAuth 로그인 | | `sym init` | 프로젝트 초기화 (.sym/ 생성) | | `sym dashboard` | 웹 대시보드 실행 (포트 8787) | | `sym my-role` | 내 역할 확인 | @@ -109,12 +101,6 @@ make lint # 린트 **필수 도구:** Go 1.21+, Node.js 18+ -## 문서 - -- [Convert 기능 가이드](docs/CONVERT_FEATURE.md) -- [Convert 사용법](docs/CONVERT_USAGE.md) -- [Linter 검증](docs/LINTER_VALIDATION.md) - ## 라이선스 MIT License diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 0e5a09b..c04de83 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/ui" "github.com/DevSymphony/sym-cli/internal/validator" @@ -80,15 +81,15 @@ func runValidate(cmd *cobra.Command, args []string) error { return fmt.Errorf("no available LLM backend for validate: %w\nTip: configure provider in .sym/config.json", err) } - var changes []validator.GitChange + var changes []git.Change if validateStaged { - changes, err = validator.GetStagedChanges() + changes, err = git.GetStagedChanges() if err != nil { return fmt.Errorf("failed to get staged changes: %w", err) } fmt.Println("Validating staged changes...") } else { - changes, err = validator.GetGitChanges() + changes, err = git.GetChanges() if err != nil { return fmt.Errorf("failed to get git changes: %w", err) } diff --git a/internal/git/README.md b/internal/git/README.md index b4d5337..28ca0e3 100644 --- a/internal/git/README.md +++ b/internal/git/README.md @@ -1,8 +1,53 @@ # git -Git 저장소 작업을 위한 유틸리티를 제공합니다. +Git 저장소의 변경 사항 조회 및 저장소 정보를 제공하는 패키지. -저장소 루트 디렉토리 탐색 및 Git 상태 조회 기능을 제공합니다. +## 패키지 구조 -**사용자**: cmd, mcp, policy, roles, server -**의존성**: 없음 +``` +git/ +├── changes.go # 변경 사항 조회 및 diff 처리 +├── changes_test.go # 테스트 +├── repo.go # 저장소 정보 조회 +└── README.md +``` + +## 의존성 + +### 패키지 사용자 + +| 패키지 | 용도 | +|--------|------| +| `internal/validator` | 검증 대상 파일 및 diff 획득 | +| `internal/cmd/validate.go` | CLI validate 명령어 | +| `internal/mcp/server.go` | MCP validate_code 도구 | +| `internal/roles` | RBAC 권한 검증 | +| `internal/policy` | 정책 관리 | + +### 패키지 의존성 + +없음. + +## Public/Private API + +### Public API + +**Types** + +| 타입 | 파일 | 설명 | +|------|------|------| +| `Change` | changes.go:10 | 파일 변경 정보 (FilePath, Status, Diff) | + +**Functions** + +| 함수 | 파일 | 설명 | +|------|------|------| +| `GetChanges()` | changes.go:18 | 모든 변경 사항 (staged + unstaged + untracked) | +| `GetStagedChanges()` | changes.go:146 | staged 변경만 | +| `ExtractAddedLines(diff)` | changes.go:185 | diff에서 추가된 줄 추출 | +| `GetRepoRoot()` | repo.go:10 | 저장소 루트 경로 | +| `GetCurrentUser()` | repo.go:21 | Git user.name | + +### Private API + +없음. diff --git a/internal/validator/git.go b/internal/git/changes.go similarity index 90% rename from internal/validator/git.go rename to internal/git/changes.go index 26ce595..b4b278b 100644 --- a/internal/validator/git.go +++ b/internal/git/changes.go @@ -1,4 +1,4 @@ -package validator +package git import ( "fmt" @@ -6,17 +6,17 @@ import ( "strings" ) -// GitChange represents a file change in git -type GitChange struct { +// Change represents a file change in git +type Change struct { FilePath string Status string // A(dded), M(odified), D(eleted) Diff string } -// GetGitChanges returns all uncommitted changes in the current git repository +// GetChanges returns all uncommitted changes in the current git repository // This includes: staged changes, unstaged changes, and untracked files -func GetGitChanges() ([]GitChange, error) { - changes := make([]GitChange, 0) +func GetChanges() ([]Change, error) { + changes := make([]Change, 0) seenFiles := make(map[string]bool) // Track files we've already processed // 1. Get staged changes (index vs HEAD) @@ -55,7 +55,7 @@ func GetGitChanges() ([]GitChange, error) { diffOutput, _ = diffCmd.Output() } - changes = append(changes, GitChange{ + changes = append(changes, Change{ FilePath: filePath, Status: status, Diff: string(diffOutput), @@ -97,7 +97,7 @@ func GetGitChanges() ([]GitChange, error) { continue } - changes = append(changes, GitChange{ + changes = append(changes, Change{ FilePath: filePath, Status: status, Diff: string(diffOutput), @@ -131,7 +131,7 @@ func GetGitChanges() ([]GitChange, error) { // We still get the output in diffOutput regardless of error _ = err // Ignore error since exit code 1 is expected for diffs - changes = append(changes, GitChange{ + changes = append(changes, Change{ FilePath: filePath, Status: "A", // Treat untracked files as Added Diff: string(diffOutput), @@ -143,7 +143,7 @@ func GetGitChanges() ([]GitChange, error) { } // GetStagedChanges returns staged changes -func GetStagedChanges() ([]GitChange, error) { +func GetStagedChanges() ([]Change, error) { cmd := exec.Command("git", "diff", "--cached", "--name-status") output, err := cmd.Output() if err != nil { @@ -152,10 +152,10 @@ func GetStagedChanges() ([]GitChange, error) { lines := strings.Split(strings.TrimSpace(string(output)), "\n") if len(lines) == 0 || lines[0] == "" { - return []GitChange{}, nil + return []Change{}, nil } - changes := make([]GitChange, 0, len(lines)) + changes := make([]Change, 0, len(lines)) for _, line := range lines { parts := strings.Fields(line) if len(parts) < 2 { @@ -171,7 +171,7 @@ func GetStagedChanges() ([]GitChange, error) { continue } - changes = append(changes, GitChange{ + changes = append(changes, Change{ FilePath: filePath, Status: status, Diff: string(diffOutput), diff --git a/internal/validator/git_test.go b/internal/git/changes_test.go similarity index 98% rename from internal/validator/git_test.go rename to internal/git/changes_test.go index 629d427..bd96f5e 100644 --- a/internal/validator/git_test.go +++ b/internal/git/changes_test.go @@ -1,4 +1,4 @@ -package validator +package git import ( "testing" diff --git a/internal/git/repo.go b/internal/git/repo.go index ae458b2..024e0de 100644 --- a/internal/git/repo.go +++ b/internal/git/repo.go @@ -3,40 +3,9 @@ package git import ( "fmt" "os/exec" - "regexp" "strings" ) -// GetRepoInfo returns the owner and repo name from the current git repository -func GetRepoInfo() (owner, repo string, err error) { - cmd := exec.Command("git", "config", "--get", "remote.origin.url") - output, err := cmd.Output() - if err != nil { - return "", "", fmt.Errorf("not a git repository or no remote origin configured") - } - - url := strings.TrimSpace(string(output)) - - // Parse various Git URL formats: - // https://github.com/owner/repo.git - // git@github.com:owner/repo.git - // https://ghes.company.com/owner/repo.git - - // SSH format: git@host:owner/repo.git - sshRegex := regexp.MustCompile(`git@[^:]+:([^/]+)/(.+?)(?:\.git)?$`) - if matches := sshRegex.FindStringSubmatch(url); len(matches) == 3 { - return matches[1], matches[2], nil - } - - // HTTPS format: https://host/owner/repo.git - httpsRegex := regexp.MustCompile(`https?://[^/]+/([^/]+)/(.+?)(?:\.git)?$`) - if matches := httpsRegex.FindStringSubmatch(url); len(matches) == 3 { - return matches[1], matches[2], nil - } - - return "", "", fmt.Errorf("could not parse repository URL: %s", url) -} - // GetRepoRoot returns the root directory of the git repository func GetRepoRoot() (string, error) { cmd := exec.Command("git", "rev-parse", "--show-toplevel") @@ -48,13 +17,6 @@ func GetRepoRoot() (string, error) { return strings.TrimSpace(string(output)), nil } -// IsGitRepo checks if the current directory is a git repository -func IsGitRepo() bool { - cmd := exec.Command("git", "rev-parse", "--git-dir") - err := cmd.Run() - return err == nil -} - // GetCurrentUser returns the current git user name func GetCurrentUser() (string, error) { cmd := exec.Command("git", "config", "--get", "user.name") diff --git a/internal/linter/pylint/converter.go b/internal/linter/pylint/converter.go index 5626236..f6cacde 100644 --- a/internal/linter/pylint/converter.go +++ b/internal/linter/pylint/converter.go @@ -139,6 +139,8 @@ Return ONLY a JSON object (no markdown fences) with this structure: "options": {"key": "value", ...} } +For string options (indent-string, good-names, bad-names, regex patterns), wrap values in single quotes: e.g. {"indent-string": "' '"} + Common Pylint rules: - Naming: invalid-name (C0103), disallowed-name (C0104) Options: variable-rgx, function-rgx, class-rgx, const-rgx, argument-rgx diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 655c5e1..492ea5a 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -448,8 +448,8 @@ func (s *Server) handleValidateCode(ctx context.Context, session *sdkmcp.ServerS var hasErrors bool // Get all git changes (staged + unstaged + untracked) - // GetGitChanges already includes all types of changes - changes, err := validator.GetGitChanges() + // GetChanges already includes all types of changes + changes, err := git.GetChanges() if err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to get git changes: %v\n", err) return map[string]interface{}{ diff --git a/internal/roles/README.md b/internal/roles/README.md index 98fd914..b933f36 100644 --- a/internal/roles/README.md +++ b/internal/roles/README.md @@ -1,8 +1,93 @@ # roles -RBAC(Role-Based Access Control)을 구현합니다. +RBAC(Role-Based Access Control) 기능을 제공합니다. -역할별 파일 접근 권한을 검증하며 glob 패턴 기반 읽기/쓰기/실행 권한 관리를 제공합니다. +역할 정의 관리와 파일 접근 권한 검증을 담당합니다. -**사용자**: cmd, server -**의존성**: git, policy +## 패키지 구조 + +``` +roles/ +├── roles.go # 역할 정의 관리 (Load/Save, Get/Set) +├── rbac.go # RBAC 권한 검증 +└── README.md +``` + +## 의존성 + +### 패키지 사용자 + +| 위치 | 용도 | +|------|------| +| `internal/cmd/init.go` | 초기 역할 생성 | +| `internal/cmd/my_role.go` | 역할 선택 CLI | +| `internal/cmd/dashboard.go` | 대시보드 역할 확인 | +| `internal/server/server.go` | 웹 대시보드 역할 API | +| `internal/mcp/server.go` | MCP RBAC 컨텍스트 | +| `internal/validator/validator.go` | 검증 시 RBAC 권한 확인 | + +### 패키지 의존성 + +``` + ┌──────────┐ + │ roles │ + └────┬─────┘ + │ + ┌─────┼─────┐ + ▼ ▼ ▼ +┌────────┐ ┌────────┐ ┌────────────┐ +│ envutil│ │ git │ │ policy │ +└────────┘ └────────┘ └────────────┘ + │ + ▼ + ┌────────────┐ + │ pkg/schema │ + └────────────┘ +``` + +## Public / Private API + +### Public API + +#### Types + +| 타입 | 파일 | 설명 | +|------|------|------| +| `Roles` | roles.go:15 | `map[string][]string` - 역할별 사용자 목록 | +| `ValidationResult` | rbac.go:15 | RBAC 검증 결과 | + +#### Functions - 역할 파일 관리 + +| 함수 | 파일 | 설명 | +|------|------|------| +| `GetRolesPath()` | roles.go:39 | .sym/roles.json 경로 반환 | +| `LoadRoles()` | roles.go:48 | roles.json 로드 | +| `SaveRoles(roles)` | roles.go:71 | roles.json 저장 | +| `RolesExists()` | roles.go:110 | roles.json 존재 여부 | + +#### Functions - 역할 상태 관리 + +| 함수 | 파일 | 설명 | +|------|------|------| +| `GetCurrentRole()` | roles.go:129 | 현재 선택된 역할 (.sym/.env) | +| `SetCurrentRole(role)` | roles.go:139 | 현재 역할 설정 | +| `GetAvailableRoles()` | roles.go:149 | 사용 가능한 역할 목록 | +| `IsValidRole(role)` | roles.go:165 | 역할 유효성 검증 | + +#### Functions - 사용자/권한 검증 + +| 함수 | 파일 | 설명 | +|------|------|------| +| `GetUserRole(username)` | roles.go:91 | 사용자의 역할 조회 | +| `LoadUserPolicyFromRepo()` | rbac.go:30 | user-policy.json 로드 | +| `ValidateFilePermissionsForRole(role, files)` | rbac.go:132 | 역할별 파일 권한 검증 | + +### Private API + +| 함수 | 파일 | 설명 | +|------|------|------| +| `getSymDir()` | roles.go:21 | .sym 디렉토리 경로 | +| `getEnvPath()` | roles.go:30 | .sym/.env 경로 | +| `getUserPolicyPath()` | rbac.go:21 | user-policy.json 경로 | +| `matchPattern(pattern, path)` | rbac.go:48 | glob 패턴 매칭 | +| `checkFilePermission(file, role)` | rbac.go:104 | 단일 파일 권한 확인 | diff --git a/internal/roles/rbac.go b/internal/roles/rbac.go index 710a029..67d48cc 100644 --- a/internal/roles/rbac.go +++ b/internal/roles/rbac.go @@ -17,8 +17,8 @@ type ValidationResult struct { DeniedFiles []string // list of files that are denied (empty if Allowed is true) } -// GetUserPolicyPath returns the path to user-policy.json in the current repo -func GetUserPolicyPath() (string, error) { +// getUserPolicyPath returns the path to user-policy.json in the current repo +func getUserPolicyPath() (string, error) { repoRoot, err := git.GetRepoRoot() if err != nil { return "", err @@ -28,7 +28,7 @@ func GetUserPolicyPath() (string, error) { // LoadUserPolicyFromRepo loads user-policy.json from the current repository func LoadUserPolicyFromRepo() (*schema.UserPolicy, error) { - policyPath, err := GetUserPolicyPath() + policyPath, err := getUserPolicyPath() if err != nil { return nil, err } @@ -125,19 +125,6 @@ func checkFilePermission(filePath string, role *schema.UserRole) bool { return false } -// ValidateFilePermissions validates if a user can modify the given files -// Returns ValidationResult with Allowed=true if all files are permitted, -// or Allowed=false with a list of denied files -func ValidateFilePermissions(username string, files []string) (*ValidationResult, error) { - // Get user's role (this internally loads roles.json) - userRole, err := GetUserRole(username) - if err != nil { - return nil, fmt.Errorf("failed to get user role: %w", err) - } - - return ValidateFilePermissionsForRole(userRole, files) -} - // ValidateFilePermissionsForRole validates if a role can modify the given files // This is the local role-based version that takes a role name directly // Returns ValidationResult with Allowed=true if all files are permitted, diff --git a/internal/roles/roles.go b/internal/roles/roles.go index b168dd3..17d8c4b 100644 --- a/internal/roles/roles.go +++ b/internal/roles/roles.go @@ -146,15 +146,6 @@ func SetCurrentRole(role string) error { return envutil.SaveKeyToEnvFile(envPath, currentRoleKey, role) } -// CurrentRoleExists checks if CURRENT_ROLE is set in .sym/.env -func CurrentRoleExists() (bool, error) { - role, err := GetCurrentRole() - if err != nil { - return false, err - } - return role != "", nil -} - // GetAvailableRoles returns all role names defined in roles.json // Returns roles sorted alphabetically for consistent ordering func GetAvailableRoles() ([]string, error) { diff --git a/internal/validator/README.md b/internal/validator/README.md index 82e8ef1..51d13ab 100644 --- a/internal/validator/README.md +++ b/internal/validator/README.md @@ -2,7 +2,105 @@ 코드 검증 오케스트레이터입니다. -정책에 정의된 규칙을 바탕으로 검증 엔진을 조율하고 검증 결과를 수집하여 위반 사항을 보고합니다. RBAC 권한 검증도 통합되어 파일 접근 권한을 확인합니다. +정책에 정의된 규칙을 바탕으로 린터와 LLM 엔진을 조율하고, 검증 결과를 수집하여 위반 사항을 보고합니다. -**사용자**: cmd, mcp -**의존성**: engine, llm, roles, git +## 패키지 구조 + +``` +validator/ +├── validator.go # Validator 구조체, 4단계 파이프라인 +├── execution_unit.go # executionUnit 인터페이스 및 구현체 +├── llm_validator.go # LLM 기반 검증 로직 +├── llm_validator_test.go +└── README.md +``` + +## 의존성 + +### 패키지 사용자 + +| 위치 | 용도 | +|------|------| +| `internal/cmd/validate.go` | sym validate CLI 명령어 | +| `internal/mcp/server.go` | MCP validate_code 도구 | + +### 패키지 의존성 + +``` + ┌─────────────┐ + │ validator │ + └──────┬──────┘ + ┌────────────┬───┴───┬────────────┐ + ▼ ▼ ▼ ▼ + ┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐ + │ linter │ │ llm │ │ roles │ │ git │ + └──────────┘ └──────────┘ └──────────┘ └──────────┘ + │ + ▼ + ┌────────────┐ + │ pkg/schema │ + └────────────┘ +``` + +## Public / Private API + +### Public API + +#### Types + +| 타입 | 파일 | 설명 | +|------|------|------| +| `Validator` | validator.go:37 | 검증 오케스트레이터 | +| `Violation` | validator.go:21 | 위반 사항 | +| `ValidationResult` | llm_validator.go:23 | 검증 결과 | +| `ValidationError` | llm_validator.go:16 | 엔진 실행 오류 | + +#### Constructors + +| 함수 | 파일 | 설명 | +|------|------|------| +| `NewValidator(policy, verbose)` | validator.go:49 | Validator 생성 | +| `NewValidatorWithWorkDir(policy, verbose, workDir)` | validator.go:71 | 커스텀 workDir로 생성 | + +#### Methods + +| 메서드 | 파일 | 설명 | +|--------|------|------| +| `(*Validator) SetLLMProvider(provider)` | validator.go:89 | LLM Provider 설정 | +| `(*Validator) ValidateChanges(ctx, changes)` | validator.go:280 | 검증 실행 | +| `(*Validator) Close()` | validator.go:413 | 리소스 정리 | + +### Private API + +#### Types + +| 타입 | 파일 | 설명 | +|------|------|------| +| `llmValidator` | llm_validator.go:35 | LLM 전용 검증기 | +| `validationResponse` | llm_validator.go:241 | LLM 응답 파싱 결과 | +| `executionUnit` | execution_unit.go:21 | 실행 단위 인터페이스 | +| `linterExecutionUnit` | execution_unit.go:34 | 린터 실행 단위 | +| `llmExecutionUnit` | execution_unit.go:216 | LLM 실행 단위 | +| `ruleGroup` | validator.go:101 | 규칙 그룹화 | + +#### Functions + +| 함수 | 파일 | 설명 | +|------|------|------| +| `newLLMValidator(provider, policy)` | llm_validator.go:41 | llmValidator 생성 | +| `getEngineName(rule)` | validator.go:93 | 규칙에서 엔진명 추출 | +| `getDefaultConcurrency()` | validator.go:109 | 기본 동시성 레벨 | +| `getLanguageFromFile(filePath)` | validator.go:420 | 파일 확장자로 언어 판별 | +| `parseValidationResponse(response)` | llm_validator.go:256 | LLM 응답 파싱 | +| `extractJSONField(response, field)` | llm_validator.go:353 | JSON 필드 추출 | + +#### Methods + +| 메서드 | 파일 | 설명 | +|--------|------|------| +| `(*Validator) groupRulesByEngine(...)` | validator.go:121 | 규칙 그룹화 | +| `(*Validator) createExecutionUnits(...)` | validator.go:179 | 실행 단위 생성 | +| `(*Validator) executeUnitsParallel(...)` | validator.go:221 | 병렬 실행 | +| `(*Validator) filterChangesForRule(...)` | validator.go:384 | 규칙별 변경 필터 | +| `(*llmValidator) Validate(...)` | llm_validator.go:54 | LLM 검증 실행 | +| `(*llmValidator) checkRule(...)` | llm_validator.go:153 | 단일 규칙 검증 | diff --git a/internal/validator/execution_unit.go b/internal/validator/execution_unit.go new file mode 100644 index 0000000..65bfe96 --- /dev/null +++ b/internal/validator/execution_unit.go @@ -0,0 +1,271 @@ +package validator + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/DevSymphony/sym-cli/internal/git" + "github.com/DevSymphony/sym-cli/internal/linter" + "github.com/DevSymphony/sym-cli/internal/llm" + "github.com/DevSymphony/sym-cli/pkg/schema" +) + +// executionUnit represents a unit of work for validation +// Each unit can be executed independently and in parallel with other units +// This interface is internal - polymorphism works within the same package regardless of visibility +type executionUnit interface { + // Execute runs the validation and returns violations + Execute(ctx context.Context) ([]Violation, error) + // GetRuleIDs returns the IDs of rules in this execution unit + GetRuleIDs() []string + // GetEngineName returns the engine name (e.g., "eslint", "pylint", "llm-validator") + GetEngineName() string + // GetFiles returns the files to be validated + GetFiles() []string +} + +// linterExecutionUnit groups multiple rules for the same linter +// All rules are validated in a single linter execution +type linterExecutionUnit struct { + engineName string + rules []schema.PolicyRule + files []string + registry *linter.Registry + symDir string + verbose bool +} + +// Execute runs the linter once with all rules and files +func (u *linterExecutionUnit) Execute(ctx context.Context) ([]Violation, error) { + if len(u.files) == 0 { + return nil, nil + } + + // Get linter from registry + lntr, err := u.registry.GetLinter(u.engineName) + if err != nil { + return nil, fmt.Errorf("linter not found: %s: %w", u.engineName, err) + } + + // Check availability and install if needed + if err := lntr.CheckAvailability(ctx); err != nil { + if u.verbose { + fmt.Printf(" 📦 Installing %s...\n", lntr.Name()) + } + if err := lntr.Install(ctx, linter.InstallConfig{ + ToolsDir: filepath.Join(os.Getenv("HOME"), ".sym", "tools"), + }); err != nil { + return nil, fmt.Errorf("failed to install %s: %w", lntr.Name(), err) + } + } + + // Get config from .sym directory (already contains all rules for this linter) + config, err := u.getConfig(lntr) + if err != nil { + return nil, fmt.Errorf("failed to get config: %w", err) + } + + // Execute linter ONCE with ALL files + startTime := time.Now() + output, err := lntr.Execute(ctx, config, u.files) + execMs := time.Since(startTime).Milliseconds() + + if err != nil { + return nil, fmt.Errorf("linter execution failed: %w", err) + } + + // Parse output to violations + linterViolations, err := lntr.ParseOutput(output) + if err != nil { + return nil, fmt.Errorf("failed to parse output: %w", err) + } + + // Map linter violations to our Violation type + violations := u.mapViolationsToRules(linterViolations, output, execMs) + + if u.verbose && output.Stdout != "" { + fmt.Printf(" 📋 %s output (%dms): %d violation(s)\n", u.engineName, execMs, len(violations)) + } + + return violations, nil +} + +// getConfig retrieves the linter configuration +func (u *linterExecutionUnit) getConfig(lntr linter.Linter) ([]byte, error) { + // Check for existing config in .sym directory + configFile := u.registry.GetConfigFile(u.engineName) + if configFile != "" { + configPath := filepath.Join(u.symDir, configFile) + if data, err := os.ReadFile(configPath); err == nil { + if u.verbose { + fmt.Printf(" 📄 Using config from %s\n", configPath) + } + return data, nil + } + } + + // Fall back to generating config from first rule + if len(u.rules) > 0 { + config := make(map[string]interface{}) + for k, val := range u.rules[0].Check { + if k != "engine" && k != "desc" { + config[k] = val + } + } + if u.rules[0].Desc != "" { + config["description"] = u.rules[0].Desc + } + return json.Marshal(config) + } + + return nil, fmt.Errorf("no config available for %s", u.engineName) +} + +// mapViolationsToRules maps linter violations back to policy rules +func (u *linterExecutionUnit) mapViolationsToRules( + linterViolations []linter.Violation, + output *linter.ToolOutput, + execMs int64, +) []Violation { + var violations []Violation + + for _, lv := range linterViolations { + // Find the matching policy rule by linter rule ID + policyRule := u.findPolicyRule(lv.RuleID) + + var policyRuleID string + var severity string + + if policyRule != nil { + policyRuleID = policyRule.ID + severity = policyRule.Severity + } else { + // If no mapping found, use a generic ID based on the linter and rule + policyRuleID = fmt.Sprintf("%s-%s", u.engineName, lv.RuleID) + // Fall back to linter severity if no policy rule found + severity = lv.Severity + } + + if severity == "" { + severity = "error" + } + + violations = append(violations, Violation{ + RuleID: policyRuleID, + Severity: severity, + Message: lv.Message, + File: lv.File, + Line: lv.Line, + Column: lv.Column, + RawOutput: output.Stdout, + RawError: output.Stderr, + ToolName: u.engineName, + ExecutionMs: execMs, + }) + } + + return violations +} + +// findPolicyRule finds the policy rule that corresponds to a linter rule ID +func (u *linterExecutionUnit) findPolicyRule(linterRuleID string) *schema.PolicyRule { + for i, rule := range u.rules { + // Check if this policy rule's check config matches the linter rule + if checkID, ok := rule.Check["ruleId"].(string); ok && checkID == linterRuleID { + return &u.rules[i] + } + // Also check if the rule ID contains the linter rule ID + if strings.Contains(rule.ID, linterRuleID) { + return &u.rules[i] + } + } + + // If no direct mapping, return the first rule (legacy behavior) + if len(u.rules) > 0 { + return &u.rules[0] + } + + return nil +} + +// GetRuleIDs returns the IDs of all rules in this execution unit +func (u *linterExecutionUnit) GetRuleIDs() []string { + ids := make([]string, len(u.rules)) + for i, rule := range u.rules { + ids[i] = rule.ID + } + return ids +} + +// GetEngineName returns the engine name +func (u *linterExecutionUnit) GetEngineName() string { + return u.engineName +} + +// GetFiles returns the files to be validated +func (u *linterExecutionUnit) GetFiles() []string { + return u.files +} + +// llmExecutionUnit represents a single (file, rule) pair for LLM validation +type llmExecutionUnit struct { + rule schema.PolicyRule + change git.Change // 단일 파일 + provider llm.Provider + policy *schema.CodePolicy + verbose bool +} + +// Execute runs the LLM validation for a single (file, rule) pair +func (u *llmExecutionUnit) Execute(ctx context.Context) ([]Violation, error) { + if u.provider == nil { + return nil, fmt.Errorf("LLM provider not configured") + } + + if u.change.Status == "D" { + return nil, nil + } + + addedLines := git.ExtractAddedLines(u.change.Diff) + if len(addedLines) == 0 && strings.TrimSpace(u.change.Diff) != "" { + addedLines = strings.Split(u.change.Diff, "\n") + } + + if len(addedLines) == 0 { + return nil, nil + } + + llmValidator := newLLMValidator(u.provider, u.policy) + violation, err := llmValidator.checkRule(ctx, u.change, addedLines, u.rule) + if err != nil { + return nil, err + } + + if violation != nil { + return []Violation{*violation}, nil + } + return nil, nil +} + +// GetRuleIDs returns the ID of this rule +func (u *llmExecutionUnit) GetRuleIDs() []string { + return []string{u.rule.ID} +} + +// GetEngineName returns "llm-validator" +func (u *llmExecutionUnit) GetEngineName() string { + return "llm-validator" +} + +// GetFiles returns the single file +func (u *llmExecutionUnit) GetFiles() []string { + if u.change.Status != "D" { + return []string{u.change.FilePath} + } + return nil +} diff --git a/internal/validator/llm_validator.go b/internal/validator/llm_validator.go index 71a0db6..13a0433 100644 --- a/internal/validator/llm_validator.go +++ b/internal/validator/llm_validator.go @@ -4,10 +4,9 @@ import ( "context" "encoding/json" "fmt" - "runtime" "strings" - "sync" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" ) @@ -28,110 +27,24 @@ type ValidationResult struct { Failed int } -// LLMValidator validates code changes against LLM-based rules. +// llmValidator validates code changes against LLM-based rules. // This validator is specifically for Git diff validation. // For regular file validation, use Validator which orchestrates all engines including LLM. -type LLMValidator struct { - provider llm.Provider - policy *schema.CodePolicy - validator *Validator +type llmValidator struct { + provider llm.Provider + policy *schema.CodePolicy } -// NewLLMValidator creates a new LLM validator -func NewLLMValidator(provider llm.Provider, policy *schema.CodePolicy) *LLMValidator { - return &LLMValidator{ - provider: provider, - policy: policy, - validator: NewValidator(policy, false), // Use main validator for orchestration +// newLLMValidator creates a new LLM validator +func newLLMValidator(provider llm.Provider, policy *schema.CodePolicy) *llmValidator { + return &llmValidator{ + provider: provider, + policy: policy, } } -// Validate validates git changes against LLM-based rules. -// This method is for diff-based validation (pre-commit hooks, PR validation). -// For regular file validation, use validator.Validate() which orchestrates all engines. -// Concurrency is limited to CPU count to prevent CPU spike. -func (v *LLMValidator) Validate(ctx context.Context, changes []GitChange) (*ValidationResult, error) { - result := &ValidationResult{ - Violations: make([]Violation, 0), - } - - // Filter rules that use llm-validator engine - llmRules := v.filterLLMRules() - if len(llmRules) == 0 { - return result, nil - } - - // Check each change against LLM rules using goroutines for parallel processing - // Limit concurrency to prevent resource exhaustion - // Use CPU/4, minimum 2, maximum 4 to balance performance and stability - var wg sync.WaitGroup - var mu sync.Mutex - - maxConcurrent := runtime.NumCPU() / 4 - if maxConcurrent < 2 { - maxConcurrent = 2 - } - if maxConcurrent > 4 { - maxConcurrent = 4 - } - sem := make(chan struct{}, maxConcurrent) - - for _, change := range changes { - if change.Status == "D" { - continue // Skip deleted files - } - - addedLines := ExtractAddedLines(change.Diff) - // If no git diff format detected, treat entire diff as code to validate - if len(addedLines) == 0 && strings.TrimSpace(change.Diff) != "" { - addedLines = strings.Split(change.Diff, "\n") - } - - if len(addedLines) == 0 { - continue - } - - // Validate against each LLM rule in parallel with concurrency limit - for _, rule := range llmRules { - mu.Lock() - result.Checked++ - mu.Unlock() - - wg.Add(1) - go func(ch GitChange, lines []string, r schema.PolicyRule) { - defer wg.Done() - - // Acquire semaphore - sem <- struct{}{} - defer func() { <-sem }() - - violation, err := v.CheckRule(ctx, ch, lines, r) - if err != nil { - // Log error but continue - fmt.Printf("Warning: failed to check rule %s: %v\n", r.ID, err) - return - } - - mu.Lock() - defer mu.Unlock() - if violation != nil { - result.Failed++ - result.Violations = append(result.Violations, *violation) - } else { - result.Passed++ - } - }(change, addedLines, rule) - } - } - - // Wait for all goroutines to complete - wg.Wait() - - return result, nil -} - // filterLLMRules filters rules that use llm-validator engine -func (v *LLMValidator) filterLLMRules() []schema.PolicyRule { +func (v *llmValidator) filterLLMRules() []schema.PolicyRule { llmRules := make([]schema.PolicyRule, 0) for _, rule := range v.policy.Rules { @@ -148,9 +61,9 @@ func (v *LLMValidator) filterLLMRules() []schema.PolicyRule { return llmRules } -// CheckRule checks if code violates a specific rule using LLM +// checkRule checks if code violates a specific rule using LLM // This is the single source of truth for LLM-based validation logic -func (v *LLMValidator) CheckRule(ctx context.Context, change GitChange, addedLines []string, rule schema.PolicyRule) (*Violation, error) { +func (v *llmValidator) checkRule(ctx context.Context, change git.Change, addedLines []string, rule schema.PolicyRule) (*Violation, error) { // Build improved prompt for LLM with clear instructions systemPrompt := `You are a strict code reviewer. Your job is to check if code changes violate a specific coding convention. diff --git a/internal/validator/llm_validator_test.go b/internal/validator/llm_validator_test.go index 49b0ab8..799201c 100644 --- a/internal/validator/llm_validator_test.go +++ b/internal/validator/llm_validator_test.go @@ -6,27 +6,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestExtractAddedLines(t *testing.T) { - diff := `diff --git a/test.go b/test.go -index 1234567..abcdefg 100644 ---- a/test.go -+++ b/test.go -@@ -1,3 +1,5 @@ - package main - -+import "fmt" -+ - func main() { -+ fmt.Println("hello") - }` - - lines := ExtractAddedLines(diff) - - assert.Len(t, lines, 3) - assert.Contains(t, lines, `import "fmt"`) - assert.Contains(t, lines, ``) - assert.Contains(t, lines, ` fmt.Println("hello")`) -} +// TestExtractAddedLines is now tested in internal/git/changes_test.go func TestParseValidationResponse_NoViolation(t *testing.T) { tests := []struct { diff --git a/internal/validator/selector.go b/internal/validator/selector.go deleted file mode 100644 index a304a10..0000000 --- a/internal/validator/selector.go +++ /dev/null @@ -1,263 +0,0 @@ -package validator - -import ( - "os" - "path/filepath" - "strings" - - "github.com/DevSymphony/sym-cli/pkg/schema" -) - -// FileSelector handles file discovery and filtering based on rule selectors -type FileSelector struct { - basePath string -} - -// NewFileSelector creates a new file selector -func NewFileSelector(basePath string) *FileSelector { - return &FileSelector{ - basePath: basePath, - } -} - -// SelectFiles finds files that match the given selector -func (fs *FileSelector) SelectFiles(selector *schema.Selector) ([]string, error) { - if selector == nil { - // No selector means match all files - return fs.findAllFiles() - } - - // Start with all files - allFiles, err := fs.findAllFiles() - if err != nil { - return nil, err - } - - // Filter by include patterns - files := allFiles - if len(selector.Include) > 0 { - files = fs.filterByPatterns(allFiles, selector.Include, true) - } - - // Filter out excluded files - if len(selector.Exclude) > 0 { - files = fs.filterByPatterns(files, selector.Exclude, false) - } - - // Filter by language (based on file extension) - if len(selector.Languages) > 0 { - files = fs.filterByLanguages(files, selector.Languages) - } - - return files, nil -} - -// findAllFiles recursively finds all files under basePath -func (fs *FileSelector) findAllFiles() ([]string, error) { - var files []string - - err := filepath.Walk(fs.basePath, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - // Skip directories - if info.IsDir() { - // Skip common directories to ignore - name := info.Name() - if name == ".git" || name == "node_modules" || name == "vendor" || name == ".sym" { - return filepath.SkipDir - } - return nil - } - - // Get relative path from base - relPath, err := filepath.Rel(fs.basePath, path) - if err != nil { - return err - } - - files = append(files, relPath) - return nil - }) - - return files, err -} - -// filterByPatterns filters files by glob patterns -func (fs *FileSelector) filterByPatterns(files []string, patterns []string, include bool) []string { - var result []string - - for _, file := range files { - matched := false - for _, pattern := range patterns { - if match, _ := filepath.Match(pattern, file); match { - matched = true - break - } - // Also try glob pattern matching (e.g., **/*.js) - if matchGlob(file, pattern) { - matched = true - break - } - } - - // Include mode: keep if matched - // Exclude mode: keep if NOT matched - if (include && matched) || (!include && !matched) { - result = append(result, file) - } - } - - return result -} - -// filterByLanguages filters files by programming language -func (fs *FileSelector) filterByLanguages(files []string, languages []string) []string { - var result []string - - extMap := buildLanguageExtensionMap(languages) - - for _, file := range files { - ext := filepath.Ext(file) - if _, ok := extMap[ext]; ok { - result = append(result, file) - } - } - - return result -} - -// buildLanguageExtensionMap creates a map of file extensions for given languages -func buildLanguageExtensionMap(languages []string) map[string]bool { - extMap := make(map[string]bool) - - for _, lang := range languages { - exts := getExtensionsForLanguage(lang) - for _, ext := range exts { - extMap[ext] = true - } - } - - return extMap -} - -// getExtensionsForLanguage returns file extensions for a language -func getExtensionsForLanguage(language string) []string { - language = strings.ToLower(language) - - switch language { - case "javascript", "js": - return []string{".js", ".mjs", ".cjs"} - case "typescript", "ts": - return []string{".ts", ".mts", ".cts"} - case "jsx": - return []string{".jsx"} - case "tsx": - return []string{".tsx"} - case "go", "golang": - return []string{".go"} - case "python", "py": - return []string{".py"} - case "java": - return []string{".java"} - case "c": - return []string{".c", ".h"} - case "cpp", "c++", "cxx": - return []string{".cpp", ".cc", ".cxx", ".hpp", ".hh", ".hxx"} - case "rust", "rs": - return []string{".rs"} - case "ruby", "rb": - return []string{".rb"} - case "php": - return []string{".php"} - case "shell", "bash", "sh": - return []string{".sh", ".bash"} - case "yaml", "yml": - return []string{".yaml", ".yml"} - case "json": - return []string{".json"} - case "xml": - return []string{".xml"} - case "html": - return []string{".html", ".htm"} - case "css": - return []string{".css"} - default: - return []string{} - } -} - -// matchGlob performs glob pattern matching with ** support -func matchGlob(path, pattern string) bool { - // Convert glob pattern to simple matching - // This is a simplified implementation - for production, use a proper glob library - - // Handle **/ prefix (matches any directory depth) - if strings.HasPrefix(pattern, "**/") { - suffix := strings.TrimPrefix(pattern, "**/") - // Check if path ends with the suffix or any subdirectory contains it - if strings.HasSuffix(path, suffix) { - return true - } - // Check if any part matches - parts := strings.Split(path, string(filepath.Separator)) - for i := range parts { - subPath := strings.Join(parts[i:], string(filepath.Separator)) - if match, _ := filepath.Match(suffix, subPath); match { - return true - } - } - } - - // Handle exact match - if match, _ := filepath.Match(pattern, path); match { - return true - } - - // Handle directory prefix patterns (e.g., "src/**") - if strings.HasSuffix(pattern, "/**") { - prefix := strings.TrimSuffix(pattern, "/**") - if strings.HasPrefix(path, prefix+string(filepath.Separator)) || path == prefix { - return true - } - } - - return false -} - -// GetLanguageFromFile determines the programming language from a file path -func GetLanguageFromFile(filePath string) string { - ext := filepath.Ext(filePath) - - switch ext { - case ".js", ".mjs", ".cjs": - return "javascript" - case ".ts", ".mts", ".cts": - return "typescript" - case ".jsx": - return "jsx" - case ".tsx": - return "tsx" - case ".go": - return "go" - case ".py": - return "python" - case ".java": - return "java" - case ".c", ".h": - return "c" - case ".cpp", ".cc", ".cxx", ".hpp", ".hh", ".hxx": - return "cpp" - case ".rs": - return "rust" - case ".rb": - return "ruby" - case ".php": - return "php" - case ".sh", ".bash": - return "shell" - default: - return "" - } -} \ No newline at end of file diff --git a/internal/validator/validator.go b/internal/validator/validator.go index 2cb47da..a17c842 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -2,14 +2,15 @@ package validator import ( "context" - "encoding/json" "fmt" "os" "path/filepath" + "runtime" "strings" "sync" "time" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/roles" @@ -34,15 +35,14 @@ type Violation struct { // Validator validates code against policy using adapters directly // This replaces the old engine-based architecture type Validator struct { - policy *schema.CodePolicy - verbose bool + policy *schema.CodePolicy + verbose bool linterRegistry *linter.Registry - workDir string - symDir string // .sym directory for config files - selector *FileSelector - ctx context.Context - ctxCancel context.CancelFunc - llmProvider llm.Provider + workDir string + symDir string // .sym directory for config files + ctx context.Context + ctxCancel context.CancelFunc + llmProvider llm.Provider } // NewValidator creates a new adapter-based validator @@ -56,15 +56,14 @@ func NewValidator(policy *schema.CodePolicy, verbose bool) *Validator { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) return &Validator{ - policy: policy, - verbose: verbose, + policy: policy, + verbose: verbose, linterRegistry: linter.Global(), - workDir: workDir, - symDir: symDir, - selector: NewFileSelector(workDir), - ctx: ctx, - ctxCancel: cancel, - llmProvider: nil, + workDir: workDir, + symDir: symDir, + ctx: ctx, + ctxCancel: cancel, + llmProvider: nil, } } @@ -75,15 +74,14 @@ func NewValidatorWithWorkDir(policy *schema.CodePolicy, verbose bool, workDir st ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) return &Validator{ - policy: policy, - verbose: verbose, + policy: policy, + verbose: verbose, linterRegistry: linter.Global(), - workDir: workDir, - symDir: symDir, - selector: NewFileSelector(workDir), - ctx: ctx, - ctxCancel: cancel, - llmProvider: nil, + workDir: workDir, + symDir: symDir, + ctx: ctx, + ctxCancel: cancel, + llmProvider: nil, } } @@ -92,260 +90,194 @@ func (v *Validator) SetLLMProvider(provider llm.Provider) { v.llmProvider = provider } -// executeRule executes a rule using the appropriate adapter -func (v *Validator) executeRule(engineName string, rule schema.PolicyRule, files []string) ([]Violation, error) { - // Special case: LLM validator - if engineName == "llm-validator" { - return v.executeLLMRule(rule, files) - } - - // Get linter directly by tool name (e.g., "eslint", "prettier", "tsc") - adp, err := v.linterRegistry.GetLinter(engineName) - if err != nil { - return nil, fmt.Errorf("adapter not found: %s: %w", engineName, err) - } - - // Check if adapter is available - if err := adp.CheckAvailability(v.ctx); err != nil { - if v.verbose { - fmt.Printf(" 📦 Installing %s...\n", adp.Name()) - } - if err := adp.Install(v.ctx, linter.InstallConfig{ - ToolsDir: filepath.Join(os.Getenv("HOME"), ".sym", "tools"), - }); err != nil { - return nil, fmt.Errorf("failed to install %s: %w", adp.Name(), err) - } - } - - // Generate config from rule or use existing .sym config - config, err := v.getAdapterConfig(adp.Name(), rule) - if err != nil { - return nil, fmt.Errorf("failed to generate config: %w", err) - } - - // Execute adapter - output, err := adp.Execute(v.ctx, config, files) - if err != nil { - return nil, fmt.Errorf("adapter execution failed: %w", err) - } - - // Parse execution duration - var execMs int64 - if output.Duration != "" { - if duration, parseErr := time.ParseDuration(output.Duration); parseErr == nil { - execMs = duration.Milliseconds() - } +// getEngineName extracts the engine name from a rule +func getEngineName(rule schema.PolicyRule) string { + if engine, ok := rule.Check["engine"].(string); ok { + return engine } + return "" +} - // Parse output to violations - adapterViolations, err := adp.ParseOutput(output) - if err != nil { - return nil, fmt.Errorf("failed to parse output: %w", err) - } +// ruleGroup groups rules by engine for batch execution +type ruleGroup struct { + engineName string + rules []schema.PolicyRule + files map[string]bool // Use set for deduplication + changes []git.Change // Original changes for LLM rules +} - // Convert adapter violations to validator violations - violations := make([]Violation, 0, len(adapterViolations)) - for _, av := range adapterViolations { - violations = append(violations, Violation{ - RuleID: rule.ID, - Severity: rule.Severity, - Message: av.Message, - File: av.File, - Line: av.Line, - Column: av.Column, - RawOutput: output.Stdout, - RawError: output.Stderr, - ToolName: adp.Name(), - ExecutionMs: execMs, - }) +// getDefaultConcurrency returns the default concurrency level (CPU/2, min 1, max 8) +func getDefaultConcurrency() int { + concurrency := runtime.NumCPU() / 2 + if concurrency < 1 { + concurrency = 1 } - - // If verbose, log the raw output - if v.verbose && output.Stdout != "" { - fmt.Printf(" 📋 Raw output (%dms):\n", execMs) - if len(output.Stdout) > 500 { - fmt.Printf(" %s...\n", output.Stdout[:500]) - } else { - fmt.Printf(" %s\n", output.Stdout) - } + if concurrency > 8 { + concurrency = 8 } - - return violations, nil + return concurrency } -// executeLLMRule executes an LLM-based rule -func (v *Validator) executeLLMRule(rule schema.PolicyRule, files []string) ([]Violation, error) { - if v.llmProvider == nil { - return nil, fmt.Errorf("LLM provider not configured") - } - - // Validate required fields for LLM validator - if rule.Desc == "" { - return nil, fmt.Errorf("LLM validator requires 'desc' field in rule %s", rule.ID) - } +// groupRulesByEngine groups rules by their engine name +func (v *Validator) groupRulesByEngine(rules []schema.PolicyRule, changes []git.Change) map[string]*ruleGroup { + groups := make(map[string]*ruleGroup) - // Check if When selector exists for file filtering - if rule.When == nil && len(files) == 0 { - if v.verbose { - fmt.Printf("⚠️ LLM rule %s has no 'when' selector and no files provided\n", rule.ID) + for _, rule := range rules { + if !rule.Enabled { + continue } - } - - // Create a consolidated ToolOutput for all files - var allResponses strings.Builder - var allErrors strings.Builder - startTime := time.Now() - totalViolations := 0 - violations := make([]Violation, 0) - - for fileIdx, file := range files { - // Read file content - content, err := os.ReadFile(file) - if err != nil { - allErrors.WriteString(fmt.Sprintf("[File %d/%d] %s: %v\n", fileIdx+1, len(files), file, err)) + engineName := getEngineName(rule) + if engineName == "" { continue } - // Build LLM prompt - systemPrompt := `You are a code reviewer. Check if the code violates the given coding convention. - -Respond with JSON only: -{ - "violates": true/false, - "description": "explanation of violation if any", - "suggestion": "how to fix it if violated" -}` - - userPrompt := fmt.Sprintf(`File: %s - -Coding Convention: -%s - -Code: -%s - -Does this code violate the convention?`, file, rule.Desc, string(content)) - - // Call LLM - fileStartTime := time.Now() - prompt := systemPrompt + "\n\n" + userPrompt - response, err := v.llmProvider.Execute(v.ctx, prompt, llm.Text) - fileExecMs := time.Since(fileStartTime).Milliseconds() - - // Record response in consolidated output - allResponses.WriteString(fmt.Sprintf("=== File %d/%d: %s (%dms) ===\n", fileIdx+1, len(files), file, fileExecMs)) - if err != nil { - allErrors.WriteString(fmt.Sprintf("[File %d/%d] LLM error: %v\n", fileIdx+1, len(files), err)) - allResponses.WriteString(fmt.Sprintf("Error: %v\n\n", err)) + // Filter changes relevant to this rule + relevantChanges := v.filterChangesForRule(changes, &rule) + if len(relevantChanges) == 0 { continue } - allResponses.WriteString(response) - allResponses.WriteString("\n\n") - - // Parse response - result := parseValidationResponse(response) - if result.Violates { - totalViolations++ - message := result.Description - if result.Suggestion != "" { - message += fmt.Sprintf(" | Suggestion: %s", result.Suggestion) + if groups[engineName] == nil { + groups[engineName] = &ruleGroup{ + engineName: engineName, + rules: []schema.PolicyRule{}, + files: make(map[string]bool), + changes: []git.Change{}, } - - // Store violation (will update with consolidated output later) - violations = append(violations, Violation{ - RuleID: rule.ID, - Severity: rule.Severity, - Message: message, - File: file, - RawOutput: response, // Individual LLM response for this file - ToolName: "llm-validator", - ExecutionMs: fileExecMs, - }) } - } - // Calculate total execution time - totalExecMs := time.Since(startTime).Milliseconds() - - // Create consolidated ToolOutput in linter format - consolidatedStdout := allResponses.String() - consolidatedStderr := allErrors.String() - - // Update all violations with consolidated output (like how linters include full output) - // Each violation gets the full stdout/stderr, just like ESLint violations all share the same JSON output - for i := range violations { - // Keep individual response in RawOutput, but add consolidated info - violations[i].RawOutput = fmt.Sprintf("=== Individual Response ===\n%s\n\n=== Consolidated Output ===\n%s", - violations[i].RawOutput, consolidatedStdout) - if consolidatedStderr != "" { - violations[i].RawError = consolidatedStderr + groups[engineName].rules = append(groups[engineName].rules, rule) + + // Collect unique files and changes + for _, change := range relevantChanges { + if change.Status != "D" { + groups[engineName].files[change.FilePath] = true + // Track changes for LLM rules (they need the diff) + if engineName == "llm-validator" { + // Check if this change is already added + found := false + for _, existing := range groups[engineName].changes { + if existing.FilePath == change.FilePath { + found = true + break + } + } + if !found { + groups[engineName].changes = append(groups[engineName].changes, change) + } + } + } } } - // If verbose, log the consolidated output (like adapter verbose output) - if v.verbose && consolidatedStdout != "" { - fmt.Printf(" 📋 LLM consolidated output (%dms):\n", totalExecMs) - fmt.Printf(" - Checked: %d file(s)\n", len(files)) - fmt.Printf(" - Violations: %d\n", totalViolations) - // Show first 500 chars of consolidated output - if len(consolidatedStdout) > 500 { - fmt.Printf(" %s...\n", consolidatedStdout[:500]) + return groups +} + +// createExecutionUnits creates execution units from rule groups +// - Linter: {모든 파일, 모든 규칙} = 1개 단위 +// - LLM: {파일 1개, 규칙 1개} = 파일수 × 규칙수 단위 +func (v *Validator) createExecutionUnits(groups map[string]*ruleGroup, changes []git.Change) []executionUnit { + var units []executionUnit + + for engineName, group := range groups { + if engineName == "llm-validator" { + // LLM: 각 (파일, 규칙) 쌍이 별도 실행 단위 + for _, rule := range group.rules { + ruleChanges := v.filterChangesForRule(group.changes, &rule) + for _, change := range ruleChanges { + if change.Status == "D" { + continue + } + units = append(units, &llmExecutionUnit{ + rule: rule, + change: change, + provider: v.llmProvider, + policy: v.policy, + verbose: v.verbose, + }) + } + } } else { - fmt.Printf(" %s\n", consolidatedStdout) - } - if consolidatedStderr != "" { - fmt.Printf(" ⚠️ Errors: %s\n", consolidatedStderr) + // Linter: 모든 파일, 모든 규칙 = 1개 실행 단위 + files := make([]string, 0, len(group.files)) + for f := range group.files { + files = append(files, f) + } + units = append(units, &linterExecutionUnit{ + engineName: engineName, + rules: group.rules, + files: files, + registry: v.linterRegistry, + symDir: v.symDir, + verbose: v.verbose, + }) } } - return violations, nil + return units } -// getAdapterConfig gets config for an adapter -// First checks .sym directory for existing config files, then generates from rule -func (v *Validator) getAdapterConfig(adapterName string, rule schema.PolicyRule) ([]byte, error) { - // Check for existing config in .sym directory (using registry) - configFile := v.linterRegistry.GetConfigFile(adapterName) - if configFile != "" { - configPath := filepath.Join(v.symDir, configFile) - if data, err := os.ReadFile(configPath); err == nil { - if v.verbose { - fmt.Printf(" 📄 Using config from %s\n", configPath) +// executeUnitsParallel executes units in parallel with semaphore-based concurrency +func (v *Validator) executeUnitsParallel(ctx context.Context, units []executionUnit) ([]Violation, []ValidationError) { + var wg sync.WaitGroup + var mu sync.Mutex + + concurrency := getDefaultConcurrency() + sem := make(chan struct{}, concurrency) + + var allViolations []Violation + var allErrors []ValidationError + + for _, unit := range units { + wg.Add(1) + go func(u executionUnit) { + defer wg.Done() + + // Acquire semaphore + select { + case sem <- struct{}{}: + case <-ctx.Done(): + mu.Lock() + allErrors = append(allErrors, ValidationError{ + RuleID: strings.Join(u.GetRuleIDs(), ","), + Engine: u.GetEngineName(), + Message: ctx.Err().Error(), + }) + mu.Unlock() + return } - return data, nil - } - } + defer func() { <-sem }() - // Otherwise, generate config from rule - config := make(map[string]interface{}) + violations, err := u.Execute(ctx) - // Copy rule.Check to config - for k, val := range rule.Check { - if k != "engine" && k != "desc" { - config[k] = val - } - } + mu.Lock() + defer mu.Unlock() - // Add rule description - if rule.Desc != "" { - config["description"] = rule.Desc + if err != nil { + allErrors = append(allErrors, ValidationError{ + RuleID: strings.Join(u.GetRuleIDs(), ","), + Engine: u.GetEngineName(), + Message: err.Error(), + }) + return + } + + allViolations = append(allViolations, violations...) + }(unit) } - return json.Marshal(config) -} + wg.Wait() -// getEngineName extracts the engine name from a rule -func getEngineName(rule schema.PolicyRule) string { - if engine, ok := rule.Check["engine"].(string); ok { - return engine - } - return "" + return allViolations, allErrors } // ValidateChanges validates git changes using adapters directly -func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (*ValidationResult, error) { +// Rules are grouped by engine for efficient batch execution: +// - Linter rules (eslint, pylint, etc.) are batched into single executions per linter +// - LLM rules are executed as individual units +// Execution units run in parallel with CPU/2 concurrency (min 1, max 8) +func (v *Validator) ValidateChanges(ctx context.Context, changes []git.Change) (*ValidationResult, error) { if v.policy == nil { return nil, fmt.Errorf("policy is not loaded") } @@ -357,7 +289,7 @@ func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (* Failed: 0, } - // Check RBAC permissions first + // Phase 1: Check RBAC permissions first if v.policy.Enforce.RBACConfig != nil && v.policy.Enforce.RBACConfig.Enabled { currentRole, err := roles.GetCurrentRole() if err == nil && currentRole != "" { @@ -391,65 +323,52 @@ func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (* } } - if v.verbose { - fmt.Printf("🔍 Validating %d change(s) against %d rule(s)...\n", len(changes), len(v.policy.Rules)) - } + // Phase 2: Group rules by engine + groups := v.groupRulesByEngine(v.policy.Rules, changes) - // Validate each enabled rule - for _, rule := range v.policy.Rules { - if !rule.Enabled { - continue - } - - engineName := getEngineName(rule) - if engineName == "" { - continue - } + // Phase 3: Create execution units + units := v.createExecutionUnits(groups, changes) - // Filter changes that match this rule's selector - relevantChanges := v.filterChangesForRule(changes, &rule) - if len(relevantChanges) == 0 { - continue + if v.verbose { + // Count files to check + totalFiles := 0 + for _, change := range changes { + if change.Status != "D" { + totalFiles++ + } } - - if v.verbose { - fmt.Printf(" Rule %s (%s): checking %d change(s)...\n", rule.ID, engineName, len(relevantChanges)) + fmt.Printf("🔍 Validating %d file(s) against %d rule(s) in %d execution unit(s) (concurrency: %d)...\n", + totalFiles, len(v.policy.Rules), len(units), getDefaultConcurrency()) + for _, unit := range units { + fmt.Printf(" - %s: %d rule(s), %d file(s)\n", + unit.GetEngineName(), len(unit.GetRuleIDs()), len(unit.GetFiles())) } + } - // For LLM engine, use parallel processing - if engineName == "llm-validator" { - v.validateLLMChanges(ctx, relevantChanges, rule, result) - } else { - // For adapter-based engines, validate files - for _, change := range relevantChanges { - if change.Status == "D" { - continue - } + // Phase 4: Execute units in parallel + violations, errors := v.executeUnitsParallel(ctx, units) - result.Checked++ - - violations, err := v.executeRule(engineName, rule, []string{change.FilePath}) - if err != nil { - // Always log errors to stderr (not just in verbose mode) - fmt.Fprintf(os.Stderr, "⚠️ Validation failed for rule %s (%s): %v\n", rule.ID, engineName, err) - // Track error in result for MCP response - result.Errors = append(result.Errors, ValidationError{ - RuleID: rule.ID, - Engine: engineName, - Message: err.Error(), - }) - continue - } + // Aggregate results + result.Violations = append(result.Violations, violations...) + result.Errors = append(result.Errors, errors...) - if len(violations) > 0 { - result.Failed++ - result.Violations = append(result.Violations, violations...) - } else { - result.Passed++ - } - } + // Calculate statistics + // Count unique files that were checked + checkedFiles := make(map[string]bool) + for _, unit := range units { + for _, file := range unit.GetFiles() { + checkedFiles[file] = true } } + result.Checked = len(checkedFiles) + + // Count failures based on violations + failedFiles := make(map[string]bool) + for _, v := range result.Violations { + failedFiles[v.File] = true + } + result.Failed = len(failedFiles) + result.Passed = result.Checked - result.Failed if v.verbose { if len(result.Violations) == 0 { @@ -462,79 +381,16 @@ func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (* return result, nil } -// validateLLMChanges validates changes using LLM in parallel -func (v *Validator) validateLLMChanges(ctx context.Context, changes []GitChange, rule schema.PolicyRule, result *ValidationResult) { - if v.llmProvider == nil { - fmt.Fprintf(os.Stderr, "⚠️ LLM provider not configured, skipping LLM validation for rule %s\n", rule.ID) - return - } - - var wg sync.WaitGroup - var mu sync.Mutex - llmValidator := NewLLMValidator(v.llmProvider, v.policy) - - for _, change := range changes { - if change.Status == "D" { - continue - } - - addedLines := ExtractAddedLines(change.Diff) - if len(addedLines) == 0 && strings.TrimSpace(change.Diff) != "" { - addedLines = strings.Split(change.Diff, "\n") - } - - if len(addedLines) == 0 { - mu.Lock() - result.Checked++ - result.Passed++ - mu.Unlock() - continue - } - - mu.Lock() - result.Checked++ - mu.Unlock() - - wg.Add(1) - go func(ch GitChange, lines []string, r schema.PolicyRule) { - defer wg.Done() - - violation, err := llmValidator.CheckRule(ctx, ch, lines, r) - if err != nil { - mu.Lock() - result.Errors = append(result.Errors, ValidationError{ - RuleID: r.ID, - Engine: "llm-validator", - Message: fmt.Sprintf("failed to check rule: %v", err), - }) - mu.Unlock() - return - } - - mu.Lock() - defer mu.Unlock() - if violation != nil { - result.Failed++ - result.Violations = append(result.Violations, *violation) - } else { - result.Passed++ - } - }(change, addedLines, rule) - } - - wg.Wait() -} - // filterChangesForRule filters git changes that match the rule's selector -func (v *Validator) filterChangesForRule(changes []GitChange, rule *schema.PolicyRule) []GitChange { +func (v *Validator) filterChangesForRule(changes []git.Change, rule *schema.PolicyRule) []git.Change { if rule.When == nil { return changes } - var filtered []GitChange + var filtered []git.Change for _, change := range changes { if len(rule.When.Languages) > 0 { - lang := GetLanguageFromFile(change.FilePath) + lang := getLanguageFromFile(change.FilePath) matched := false for _, ruleLang := range rule.When.Languages { if ruleLang == lang { @@ -560,3 +416,39 @@ func (v *Validator) Close() error { } return nil } + +// getLanguageFromFile determines the programming language from a file path +func getLanguageFromFile(filePath string) string { + ext := filepath.Ext(filePath) + + switch ext { + case ".js", ".mjs", ".cjs": + return "javascript" + case ".ts", ".mts", ".cts": + return "typescript" + case ".jsx": + return "jsx" + case ".tsx": + return "tsx" + case ".go": + return "go" + case ".py": + return "python" + case ".java": + return "java" + case ".c", ".h": + return "c" + case ".cpp", ".cc", ".cxx", ".hpp", ".hh", ".hxx": + return "cpp" + case ".rs": + return "rust" + case ".rb": + return "ruby" + case ".php": + return "php" + case ".sh", ".bash": + return "shell" + default: + return "" + } +} diff --git a/internal/validator/validator_test.go b/internal/validator/validator_test.go new file mode 100644 index 0000000..6604e0d --- /dev/null +++ b/internal/validator/validator_test.go @@ -0,0 +1,517 @@ +package validator + +import ( + "testing" + + "github.com/DevSymphony/sym-cli/internal/git" + "github.com/DevSymphony/sym-cli/internal/linter" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" +) + +func TestGetEngineName(t *testing.T) { + tests := []struct { + name string + rule schema.PolicyRule + expected string + }{ + { + name: "valid engine", + rule: schema.PolicyRule{ + Check: map[string]interface{}{"engine": "eslint"}, + }, + expected: "eslint", + }, + { + name: "missing engine", + rule: schema.PolicyRule{ + Check: map[string]interface{}{"other": "value"}, + }, + expected: "", + }, + { + name: "non-string engine", + rule: schema.PolicyRule{ + Check: map[string]interface{}{"engine": 123}, + }, + expected: "", + }, + { + name: "nil check map", + rule: schema.PolicyRule{ + Check: nil, + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getEngineName(tt.rule) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetDefaultConcurrency(t *testing.T) { + result := getDefaultConcurrency() + assert.GreaterOrEqual(t, result, 1, "concurrency should be at least 1") + assert.LessOrEqual(t, result, 8, "concurrency should be at most 8") +} + +func TestGetLanguageFromFile(t *testing.T) { + tests := []struct { + name string + filePath string + expected string + }{ + // JavaScript + {"js file", "app.js", "javascript"}, + {"mjs file", "module.mjs", "javascript"}, + {"cjs file", "common.cjs", "javascript"}, + // TypeScript + {"ts file", "app.ts", "typescript"}, + {"mts file", "module.mts", "typescript"}, + {"cts file", "common.cts", "typescript"}, + // JSX/TSX + {"jsx file", "component.jsx", "jsx"}, + {"tsx file", "component.tsx", "tsx"}, + // Go + {"go file", "main.go", "go"}, + // Python + {"py file", "script.py", "python"}, + // Java + {"java file", "Main.java", "java"}, + // C + {"c file", "main.c", "c"}, + {"h file", "header.h", "c"}, + // C++ + {"cpp file", "main.cpp", "cpp"}, + {"cc file", "main.cc", "cpp"}, + {"cxx file", "main.cxx", "cpp"}, + {"hpp file", "header.hpp", "cpp"}, + {"hh file", "header.hh", "cpp"}, + {"hxx file", "header.hxx", "cpp"}, + // Rust + {"rs file", "main.rs", "rust"}, + // Ruby + {"rb file", "script.rb", "ruby"}, + // PHP + {"php file", "index.php", "php"}, + // Shell + {"sh file", "script.sh", "shell"}, + {"bash file", "script.bash", "shell"}, + // Unknown/unsupported + {"unknown extension", "file.xyz", ""}, + {"no extension", "Makefile", ""}, + // Path with directories + {"nested path", "src/components/Button.tsx", "tsx"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getLanguageFromFile(tt.filePath) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestParseJSON(t *testing.T) { + t.Run("valid JSON", func(t *testing.T) { + var target jsonValidationResponse + err := parseJSON(`{"violates": true, "confidence": "high"}`, &target) + assert.NoError(t, err) + assert.True(t, target.Violates) + assert.Equal(t, "high", target.Confidence) + }) + + t.Run("invalid JSON", func(t *testing.T) { + var target jsonValidationResponse + err := parseJSON(`{invalid json}`, &target) + assert.Error(t, err) + }) + + t.Run("empty JSON", func(t *testing.T) { + var target jsonValidationResponse + err := parseJSON(`{}`, &target) + assert.NoError(t, err) + assert.False(t, target.Violates) + }) +} + +func TestParseValidationResponseFallback(t *testing.T) { + tests := []struct { + name string + response string + expectViolates bool + expectConfidence string + }{ + { + name: "violates true", + response: `{"violates": true, "description": "test"}`, + expectViolates: true, + expectConfidence: "medium", + }, + { + name: "violates true no space", + response: `{"violates":true}`, + expectViolates: true, + expectConfidence: "medium", + }, + { + name: "violates false", + response: `{"violates": false}`, + expectViolates: false, + expectConfidence: "low", + }, + { + name: "does not violate text", + response: `The code does not violate the rule.`, + expectViolates: false, + expectConfidence: "low", + }, + { + name: "no violation indicators", + response: `Random text without any violation`, + expectViolates: false, + expectConfidence: "low", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseValidationResponseFallback(tt.response) + assert.Equal(t, tt.expectViolates, result.Violates) + assert.Equal(t, tt.expectConfidence, result.Confidence) + }) + } +} + +func TestParseValidationResponseFallback_WithDescriptionExtraction(t *testing.T) { + response := `{"violates": true, "description": "Found security issue", "suggestion": "Fix it"}` + result := parseValidationResponseFallback(response) + + assert.True(t, result.Violates) + assert.Equal(t, "Found security issue", result.Description) + assert.Equal(t, "Fix it", result.Suggestion) +} + +func TestFilterChangesForRule(t *testing.T) { + changes := []git.Change{ + {FilePath: "main.go", Status: "M"}, + {FilePath: "app.py", Status: "M"}, + {FilePath: "script.js", Status: "A"}, + } + + v := &Validator{} + + t.Run("nil When selector returns all", func(t *testing.T) { + rule := &schema.PolicyRule{When: nil} + result := v.filterChangesForRule(changes, rule) + assert.Len(t, result, 3) + }) + + t.Run("empty When selector returns all", func(t *testing.T) { + rule := &schema.PolicyRule{When: &schema.Selector{}} + result := v.filterChangesForRule(changes, rule) + assert.Len(t, result, 3) + }) + + t.Run("filter by language - go only", func(t *testing.T) { + rule := &schema.PolicyRule{ + When: &schema.Selector{Languages: []string{"go"}}, + } + result := v.filterChangesForRule(changes, rule) + assert.Len(t, result, 1) + assert.Equal(t, "main.go", result[0].FilePath) + }) + + t.Run("filter by language - multiple languages", func(t *testing.T) { + rule := &schema.PolicyRule{ + When: &schema.Selector{Languages: []string{"go", "python"}}, + } + result := v.filterChangesForRule(changes, rule) + assert.Len(t, result, 2) + }) + + t.Run("filter by language - no match", func(t *testing.T) { + rule := &schema.PolicyRule{ + When: &schema.Selector{Languages: []string{"rust"}}, + } + result := v.filterChangesForRule(changes, rule) + assert.Len(t, result, 0) + }) +} + +func TestFilterLLMRules_Detailed(t *testing.T) { + t.Run("filters llm-validator rules only", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "rule1", Enabled: true, Check: map[string]interface{}{"engine": "llm-validator"}}, + {ID: "rule2", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + {ID: "rule3", Enabled: true, Check: map[string]interface{}{"engine": "llm-validator"}}, + }, + } + v := &llmValidator{policy: policy} + result := v.filterLLMRules() + assert.Len(t, result, 2) + assert.Equal(t, "rule1", result[0].ID) + assert.Equal(t, "rule3", result[1].ID) + }) + + t.Run("skips disabled rules", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "rule1", Enabled: false, Check: map[string]interface{}{"engine": "llm-validator"}}, + {ID: "rule2", Enabled: true, Check: map[string]interface{}{"engine": "llm-validator"}}, + }, + } + v := &llmValidator{policy: policy} + result := v.filterLLMRules() + assert.Len(t, result, 1) + assert.Equal(t, "rule2", result[0].ID) + }) + + t.Run("returns empty for no matching rules", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "rule1", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + }, + } + v := &llmValidator{policy: policy} + result := v.filterLLMRules() + assert.Len(t, result, 0) + }) +} + +func TestLinterExecutionUnit_Getters(t *testing.T) { + rules := []schema.PolicyRule{ + {ID: "rule-1"}, + {ID: "rule-2"}, + } + files := []string{"file1.js", "file2.js"} + + unit := &linterExecutionUnit{ + engineName: "eslint", + rules: rules, + files: files, + } + + t.Run("GetRuleIDs", func(t *testing.T) { + ids := unit.GetRuleIDs() + assert.Equal(t, []string{"rule-1", "rule-2"}, ids) + }) + + t.Run("GetEngineName", func(t *testing.T) { + assert.Equal(t, "eslint", unit.GetEngineName()) + }) + + t.Run("GetFiles", func(t *testing.T) { + assert.Equal(t, files, unit.GetFiles()) + }) +} + +func TestLlmExecutionUnit_Getters(t *testing.T) { + rule := schema.PolicyRule{ID: "llm-rule-1"} + + t.Run("normal file", func(t *testing.T) { + unit := &llmExecutionUnit{ + rule: rule, + change: git.Change{FilePath: "test.py", Status: "M"}, + } + assert.Equal(t, []string{"llm-rule-1"}, unit.GetRuleIDs()) + assert.Equal(t, "llm-validator", unit.GetEngineName()) + assert.Equal(t, []string{"test.py"}, unit.GetFiles()) + }) + + t.Run("deleted file returns nil files", func(t *testing.T) { + unit := &llmExecutionUnit{ + rule: rule, + change: git.Change{FilePath: "deleted.py", Status: "D"}, + } + assert.Nil(t, unit.GetFiles()) + }) +} + +func TestFindPolicyRule(t *testing.T) { + rules := []schema.PolicyRule{ + {ID: "rule-1", Check: map[string]interface{}{"ruleId": "no-console"}}, + {ID: "rule-2", Check: map[string]interface{}{"ruleId": "no-unused-vars"}}, + {ID: "eslint-no-debugger"}, + } + unit := &linterExecutionUnit{rules: rules} + + t.Run("match by ruleId in Check", func(t *testing.T) { + result := unit.findPolicyRule("no-console") + assert.NotNil(t, result) + assert.Equal(t, "rule-1", result.ID) + }) + + t.Run("match by rule ID contains", func(t *testing.T) { + result := unit.findPolicyRule("no-debugger") + assert.NotNil(t, result) + assert.Equal(t, "eslint-no-debugger", result.ID) + }) + + t.Run("fallback to first rule", func(t *testing.T) { + result := unit.findPolicyRule("unknown-rule") + assert.NotNil(t, result) + assert.Equal(t, "rule-1", result.ID) + }) + + t.Run("empty rules returns nil", func(t *testing.T) { + emptyUnit := &linterExecutionUnit{rules: []schema.PolicyRule{}} + result := emptyUnit.findPolicyRule("any-rule") + assert.Nil(t, result) + }) +} + +func TestMapViolationsToRules(t *testing.T) { + rules := []schema.PolicyRule{ + {ID: "policy-rule-1", Severity: "warning", Check: map[string]interface{}{"ruleId": "no-console"}}, + } + unit := &linterExecutionUnit{ + engineName: "eslint", + rules: rules, + } + output := &linter.ToolOutput{ + Stdout: "test output", + Stderr: "", + } + + t.Run("maps with matching policy rule", func(t *testing.T) { + linterViolations := []linter.Violation{ + {File: "app.js", Line: 10, Column: 5, Message: "Unexpected console", RuleID: "no-console"}, + } + result := unit.mapViolationsToRules(linterViolations, output, 100) + + assert.Len(t, result, 1) + assert.Equal(t, "policy-rule-1", result[0].RuleID) + assert.Equal(t, "warning", result[0].Severity) + assert.Equal(t, "app.js", result[0].File) + assert.Equal(t, 10, result[0].Line) + assert.Equal(t, "eslint", result[0].ToolName) + assert.Equal(t, int64(100), result[0].ExecutionMs) + }) + + t.Run("maps without matching rule uses fallback", func(t *testing.T) { + linterViolations := []linter.Violation{ + {File: "app.js", Line: 1, Message: "Some error", RuleID: "unknown-rule", Severity: "error"}, + } + result := unit.mapViolationsToRules(linterViolations, output, 50) + + assert.Len(t, result, 1) + assert.Equal(t, "policy-rule-1", result[0].RuleID) // fallback to first rule + }) + + t.Run("empty severity defaults to error", func(t *testing.T) { + emptyRulesUnit := &linterExecutionUnit{ + engineName: "eslint", + rules: []schema.PolicyRule{}, + } + linterViolations := []linter.Violation{ + {File: "app.js", Line: 1, Message: "Error", RuleID: "some-rule", Severity: ""}, + } + result := emptyRulesUnit.mapViolationsToRules(linterViolations, output, 10) + + assert.Len(t, result, 1) + assert.Equal(t, "error", result[0].Severity) + }) +} + +func TestGroupRulesByEngine(t *testing.T) { + changes := []git.Change{ + {FilePath: "app.js", Status: "M"}, + {FilePath: "main.go", Status: "A"}, + } + + t.Run("groups rules by engine", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "r1", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + {ID: "r2", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + {ID: "r3", Enabled: true, Check: map[string]interface{}{"engine": "golint"}}, + }, + } + v := &Validator{policy: policy} + groups := v.groupRulesByEngine(policy.Rules, changes) + + assert.Len(t, groups, 2) + assert.Len(t, groups["eslint"].rules, 2) + assert.Len(t, groups["golint"].rules, 1) + }) + + t.Run("skips disabled rules", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "r1", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + {ID: "r2", Enabled: false, Check: map[string]interface{}{"engine": "eslint"}}, + }, + } + v := &Validator{policy: policy} + groups := v.groupRulesByEngine(policy.Rules, changes) + + assert.Len(t, groups["eslint"].rules, 1) + }) + + t.Run("skips rules without engine", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "r1", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + {ID: "r2", Enabled: true, Check: map[string]interface{}{}}, + }, + } + v := &Validator{policy: policy} + groups := v.groupRulesByEngine(policy.Rules, changes) + + assert.Len(t, groups, 1) + assert.Len(t, groups["eslint"].rules, 1) + }) + + t.Run("deduplicates files", func(t *testing.T) { + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "r1", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + {ID: "r2", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + }, + } + v := &Validator{policy: policy} + groups := v.groupRulesByEngine(policy.Rules, changes) + + // Both rules apply to same files, should be deduplicated + assert.Len(t, groups["eslint"].files, 2) // app.js and main.go + }) + + t.Run("tracks changes for llm-validator", func(t *testing.T) { + llmChanges := []git.Change{ + {FilePath: "app.js", Status: "M", Diff: "diff content"}, + } + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "r1", Enabled: true, Check: map[string]interface{}{"engine": "llm-validator"}}, + }, + } + v := &Validator{policy: policy} + groups := v.groupRulesByEngine(policy.Rules, llmChanges) + + assert.Len(t, groups["llm-validator"].changes, 1) + assert.Equal(t, "app.js", groups["llm-validator"].changes[0].FilePath) + }) + + t.Run("skips deleted files", func(t *testing.T) { + deletedChanges := []git.Change{ + {FilePath: "app.js", Status: "D"}, + {FilePath: "main.go", Status: "M"}, + } + policy := &schema.CodePolicy{ + Rules: []schema.PolicyRule{ + {ID: "r1", Enabled: true, Check: map[string]interface{}{"engine": "eslint"}}, + }, + } + v := &Validator{policy: policy} + groups := v.groupRulesByEngine(policy.Rules, deletedChanges) + + assert.Len(t, groups["eslint"].files, 1) + assert.True(t, groups["eslint"].files["main.go"]) + }) +} diff --git a/npm/package.json b/npm/package.json index 94522a4..dbf6ef6 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "@dev-symphony/sym", - "version": "0.1.6", + "version": "0.1.7", "description": "Symphony - LLM-friendly convention linter for AI coding assistants", "keywords": [ "mcp", diff --git a/tests/e2e/full_workflow_test.go b/tests/e2e/full_workflow_test.go index b1a9e83..ff74177 100644 --- a/tests/e2e/full_workflow_test.go +++ b/tests/e2e/full_workflow_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/DevSymphony/sym-cli/internal/converter" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -168,18 +169,20 @@ func ProcessData(data string) error { // ========== STEP 4: Validate generated code ========== t.Log("STEP 4: Validating generated code against conventions") - llmValidator := validator.NewLLMValidator(provider, &convertedPolicy) + llmValidator := validator.NewValidator(&convertedPolicy, false) + llmValidator.SetLLMProvider(provider) + defer llmValidator.Close() // Validate BAD code t.Log("STEP 4a: Validating BAD code (should find violations)") - badChanges := []validator.GitChange{ + badChanges := []git.Change{ { FilePath: badCodePath, Diff: badGeneratedCode, }, } - badResult, err := llmValidator.Validate(ctx, badChanges) + badResult, err := llmValidator.ValidateChanges(ctx, badChanges) require.NoError(t, err, "Validation should not error") t.Logf("✓ Validation completed: checked=%d, violations=%d", @@ -204,14 +207,14 @@ func ProcessData(data string) error { // Validate GOOD code t.Log("STEP 4b: Validating GOOD code (should pass or have fewer violations)") - goodChanges := []validator.GitChange{ + goodChanges := []git.Change{ { FilePath: goodCodePath, Diff: goodGeneratedCode, }, } - goodResult, err := llmValidator.Validate(ctx, goodChanges) + goodResult, err := llmValidator.ValidateChanges(ctx, goodChanges) require.NoError(t, err) t.Logf("✓ Validation completed: checked=%d, violations=%d", @@ -335,14 +338,16 @@ func TestE2E_CodeGenerationFeedbackLoop(t *testing.T) { cfg := llm.LoadConfig() provider, err := llm.New(cfg) require.NoError(t, err, "LLM provider creation should succeed") - v := validator.NewLLMValidator(provider, policy) + v := validator.NewValidator(policy, false) + v.SetLLMProvider(provider) + defer v.Close() ctx := context.Background() // Iteration 1: Bad code t.Log("Iteration 1: Validating initial code with violations") iteration1 := `+const APIKey = "sk-test123"` - result1, err := v.Validate(ctx, []validator.GitChange{ + result1, err := v.ValidateChanges(ctx, []git.Change{ {FilePath: "test.go", Diff: iteration1}, }) require.NoError(t, err) @@ -355,7 +360,7 @@ func TestE2E_CodeGenerationFeedbackLoop(t *testing.T) { t.Log("Iteration 2: Validating fixed code") iteration2 := `+apiKey := os.Getenv("API_KEY")` - result2, err := v.Validate(ctx, []validator.GitChange{ + result2, err := v.ValidateChanges(ctx, []git.Change{ {FilePath: "test.go", Diff: iteration2}, }) require.NoError(t, err) diff --git a/tests/e2e/mcp_integration_test.go b/tests/e2e/mcp_integration_test.go index f955632..b7ebfa2 100644 --- a/tests/e2e/mcp_integration_test.go +++ b/tests/e2e/mcp_integration_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -145,7 +146,9 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { require.NoError(t, err, "LLM provider creation should succeed") // Create validator - v := validator.NewLLMValidator(provider, policy) + v := validator.NewValidator(policy, false) + v.SetLLMProvider(provider) + defer v.Close() ctx := context.Background() // Test 1: Validate BAD code (should find multiple violations) @@ -162,7 +165,7 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { } formattedDiff := strings.Join(diffLines, "\n") - changes := []validator.GitChange{ + changes := []git.Change{ { FilePath: "examples/bad-example.js", Diff: formattedDiff, @@ -170,7 +173,7 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { } t.Log("Validating bad code against conventions...") - result, err := v.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) require.NoError(t, err, "Validation should not error") t.Logf("Validation completed: checked=%d, violations=%d", @@ -218,7 +221,7 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { } formattedDiff := strings.Join(diffLines, "\n") - changes := []validator.GitChange{ + changes := []git.Change{ { FilePath: "examples/good-example.js", Diff: formattedDiff, @@ -226,7 +229,7 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { } t.Log("Validating good code against conventions...") - result, err := v.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) require.NoError(t, err) t.Logf("Validation completed: checked=%d, violations=%d", @@ -256,7 +259,9 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { Rules: filterRulesByCategory(policy.Rules, "security"), } - securityValidator := validator.NewLLMValidator(provider, securityPolicy) + securityValidator := validator.NewValidator(securityPolicy, false) + securityValidator.SetLLMProvider(provider) + defer securityValidator.Close() // Code with security violation (format as git diff with + prefix) codeWithSecurityIssue := `+const apiKey = "sk-1234567890abcdef"; // Hardcoded secret @@ -264,14 +269,14 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { + headers: { 'Authorization': 'Bearer ' + apiKey } +});` - changes := []validator.GitChange{ + changes := []git.Change{ { FilePath: "test-security.js", Diff: codeWithSecurityIssue, }, } - result, err := securityValidator.Validate(ctx, changes) + result, err := securityValidator.ValidateChanges(ctx, changes) require.NoError(t, err) t.Logf("Security validation: checked=%d, violations=%d", @@ -295,7 +300,7 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { // Iteration 1: Code with hardcoded secret (format with + prefix) iteration1 := `+const apiKey = "sk-test123";` - result1, err := v.Validate(ctx, []validator.GitChange{ + result1, err := v.ValidateChanges(ctx, []git.Change{ {FilePath: "test.js", Diff: iteration1}, }) require.NoError(t, err) @@ -305,7 +310,7 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { // Iteration 2: AI fixes the issue (format with + prefix) iteration2 := `+const apiKey = process.env.API_KEY;` - result2, err := v.Validate(ctx, []validator.GitChange{ + result2, err := v.ValidateChanges(ctx, []git.Change{ {FilePath: "test.js", Diff: iteration2}, }) require.NoError(t, err) @@ -380,9 +385,11 @@ func TestMCP_EndToEndWorkflow(t *testing.T) { llmCfg := llm.LoadConfig() llmProvider, err := llm.New(llmCfg) require.NoError(t, err, "LLM provider creation should succeed") - v := validator.NewLLMValidator(llmProvider, policy) + v := validator.NewValidator(policy, false) + v.SetLLMProvider(llmProvider) + defer v.Close() - result, err := v.Validate(context.Background(), []validator.GitChange{ + result, err := v.ValidateChanges(context.Background(), []git.Change{ {FilePath: "auth.js", Diff: generatedCode}, }) require.NoError(t, err) diff --git a/tests/e2e/unit_workflow_test.go b/tests/e2e/unit_workflow_test.go index c278d1e..7b7b330 100644 --- a/tests/e2e/unit_workflow_test.go +++ b/tests/e2e/unit_workflow_test.go @@ -43,56 +43,9 @@ func TestUnit_PolicyParsing(t *testing.T) { } // TestUnit_GitDiffExtraction tests extracting added lines from git diff +// Note: extractAddedLines is now internal to the validator package func TestUnit_GitDiffExtraction(t *testing.T) { - tests := []struct { - name string - diff string - expected []string - }{ - { - name: "simple addition", - diff: `diff --git a/test.go b/test.go -@@ -1,2 +1,3 @@ - package main -+const APIKey = "secret" - func main() {}`, - expected: []string{`const APIKey = "secret"`}, - }, - { - name: "multiple additions", - diff: `diff --git a/test.go b/test.go -@@ -1,2 +1,5 @@ - package main -+import "os" -+ -+const APIKey = "secret" - func main() {}`, - expected: []string{`import "os"`, ``, `const APIKey = "secret"`}, - }, - { - name: "no additions", - diff: `diff --git a/test.go b/test.go -@@ -1,3 +1,2 @@ - package main --const APIKey = "secret" - func main() {}`, - expected: []string{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - lines := validator.ExtractAddedLines(tt.diff) - - assert.Equal(t, len(tt.expected), len(lines), - "Should extract correct number of lines") - - for _, expectedLine := range tt.expected { - assert.Contains(t, lines, expectedLine, - "Should contain: %s", expectedLine) - } - }) - } + t.Skip("extractAddedLines is now internal - tested via internal/validator/git_test.go") } // TestUnit_RuleFiltering tests filtering rules by category and severity diff --git a/tests/e2e/validator_test.go b/tests/e2e/validator_test.go index f5bc81f..b659be0 100644 --- a/tests/e2e/validator_test.go +++ b/tests/e2e/validator_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -36,10 +37,12 @@ func TestE2E_ValidatorWithPolicy(t *testing.T) { require.NoError(t, err, "LLM provider creation should succeed") // Create validator - v := validator.NewLLMValidator(provider, policy) + v := validator.NewValidator(policy, false) + v.SetLLMProvider(provider) + defer v.Close() // Create a test change (simulating git diff output) - changes := []validator.GitChange{ + changes := []git.Change{ { FilePath: "tests/scenario/bad_code.go", Diff: `+const APIKey = "sk-1234567890abcdefghijklmnopqrstuvwxyz"`, @@ -48,7 +51,7 @@ func TestE2E_ValidatorWithPolicy(t *testing.T) { // Run validation ctx := context.Background() - result, err := v.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) // Assertions require.NoError(t, err, "Validation should not error") @@ -90,10 +93,12 @@ func TestE2E_ValidatorWithGoodCode(t *testing.T) { require.NoError(t, err, "LLM provider creation should succeed") // Create validator - v := validator.NewLLMValidator(provider, policy) + v := validator.NewValidator(policy, false) + v.SetLLMProvider(provider) + defer v.Close() // Create a test change with good code - changes := []validator.GitChange{ + changes := []git.Change{ { FilePath: "tests/scenario/good_code.go", Diff: `+var APIKey = os.Getenv("OPENAI_API_KEY")`, @@ -102,7 +107,7 @@ func TestE2E_ValidatorWithGoodCode(t *testing.T) { // Run validation ctx := context.Background() - result, err := v.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) // Assertions require.NoError(t, err) @@ -114,27 +119,11 @@ func TestE2E_ValidatorWithGoodCode(t *testing.T) { } // TestE2E_GitChangeExtraction tests git diff extraction +// Note: extractAddedLines is now internal, testing via ValidateChanges func TestE2E_GitChangeExtraction(t *testing.T) { - // This test doesn't need API key - diff := `diff --git a/test.go b/test.go -index 1234567..abcdefg 100644 ---- a/test.go -+++ b/test.go -@@ -1,3 +1,5 @@ - package main - -+const APIKey = "sk-test123" -+ - func main() { -+ println(APIKey) - }` - - lines := validator.ExtractAddedLines(diff) - - // Should extract only added lines - assert.Contains(t, lines, `const APIKey = "sk-test123"`) - assert.Contains(t, lines, ``) - assert.Contains(t, lines, ` println(APIKey)`) + // This test verifies that git diff format is correctly processed + // by the validator (internally uses extractAddedLines) + t.Skip("extractAddedLines is now internal - tested via ValidateChanges") } // TestE2E_PolicyParsing tests policy file parsing @@ -191,10 +180,12 @@ func TestE2E_ValidatorFilter(t *testing.T) { require.NoError(t, err, "LLM provider creation should succeed") // Create validator - v := validator.NewLLMValidator(provider, policy) + v := validator.NewValidator(policy, false) + v.SetLLMProvider(provider) + defer v.Close() // Test with Go file - changes := []validator.GitChange{ + changes := []git.Change{ { FilePath: "test.go", Diff: "+const x = 1", @@ -202,7 +193,7 @@ func TestE2E_ValidatorFilter(t *testing.T) { } ctx := context.Background() - result, err := v.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) require.NoError(t, err) assert.NotNil(t, result) diff --git a/tests/integration/checkstyle_test.go b/tests/integration/checkstyle_test.go index 7b108e7..b9955e3 100644 --- a/tests/integration/checkstyle_test.go +++ b/tests/integration/checkstyle_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -54,7 +55,7 @@ func TestCheckstyle_ValidateChanges(t *testing.T) { testFile := filepath.Join(testdataDir, "Test.java") require.FileExists(t, testFile, "Test.java should exist") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -123,7 +124,7 @@ func TestCheckstyle_NamingRules(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.java") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -210,7 +211,7 @@ func TestCheckstyle_ToolNameAndRuleID(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.java") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", diff --git a/tests/integration/eslint_test.go b/tests/integration/eslint_test.go index 46ef381..ebdb3ee 100644 --- a/tests/integration/eslint_test.go +++ b/tests/integration/eslint_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -54,7 +55,7 @@ func TestESLint_ValidateChanges(t *testing.T) { testFile := filepath.Join(testdataDir, "Test.ts") require.FileExists(t, testFile, "Test.ts should exist") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -122,7 +123,7 @@ func TestESLint_NamingConventions(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -179,7 +180,7 @@ func TestESLint_MaxLineLength(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -234,7 +235,7 @@ func TestESLint_ToolNameAndRuleID(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", diff --git a/tests/integration/pmd_test.go b/tests/integration/pmd_test.go index 34b4c21..79f6af0 100644 --- a/tests/integration/pmd_test.go +++ b/tests/integration/pmd_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -54,7 +55,7 @@ func TestPMD_ValidateChanges(t *testing.T) { testFile := filepath.Join(testdataDir, "Test.java") require.FileExists(t, testFile, "Test.java should exist") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -123,7 +124,7 @@ func TestPMD_EmptyCatchBlock(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.java") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -178,7 +179,7 @@ func TestPMD_UnusedPrivateMethod(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.java") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -233,7 +234,7 @@ func TestPMD_ToolNameAndRuleID(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.java") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", diff --git a/tests/integration/prettier_test.go b/tests/integration/prettier_test.go index c657837..a00e178 100644 --- a/tests/integration/prettier_test.go +++ b/tests/integration/prettier_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -54,7 +55,7 @@ func TestPrettier_ValidateChanges(t *testing.T) { testFile := filepath.Join(testdataDir, "Test.ts") require.FileExists(t, testFile, "Test.ts should exist") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -123,7 +124,7 @@ func TestPrettier_FormattingCheck(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -190,7 +191,7 @@ func TestPrettier_QuotesAndIndent(t *testing.T) { // Verify the file contains double quotes (which should be single) assert.Contains(t, string(content), `"https://`, "Test file should contain double quotes") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -236,7 +237,7 @@ func TestPrettier_ToolNameAndRuleID(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", diff --git a/tests/integration/pylint_test.go b/tests/integration/pylint_test.go index d9ac580..ebd3f2e 100644 --- a/tests/integration/pylint_test.go +++ b/tests/integration/pylint_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -54,7 +55,7 @@ func TestPylint_ValidateChanges(t *testing.T) { testFile := filepath.Join(testdataDir, "Test.py") require.FileExists(t, testFile, "Test.py should exist") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", // Modified Diff: "", // Diff not needed for adapter-based rules @@ -124,7 +125,7 @@ func TestPylint_NamingConventions(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.py") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -210,7 +211,7 @@ func TestPylint_ToolNameAndRuleID(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.py") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", diff --git a/tests/integration/rbac_test.go b/tests/integration/rbac_test.go index ebc01ad..53f6076 100644 --- a/tests/integration/rbac_test.go +++ b/tests/integration/rbac_test.go @@ -14,14 +14,14 @@ func TestComplexRBACPatterns(t *testing.T) { tests := []struct { name string - username string + role string files []string expectAllow bool expectDenied []string }{ { - name: "Admin can modify all files", - username: "alice", // alice is admin + name: "Admin can modify all files", + role: "admin", files: []string{ "src/components/Button.js", "src/core/engine.js", @@ -32,8 +32,8 @@ func TestComplexRBACPatterns(t *testing.T) { expectDenied: []string{}, }, { - name: "Developer can modify source files", - username: "charlie", // charlie is developer + name: "Developer can modify source files", + role: "developer", files: []string{ "src/components/Button.js", "src/components/ui/Modal.js", @@ -43,8 +43,8 @@ func TestComplexRBACPatterns(t *testing.T) { expectDenied: []string{}, }, { - name: "Developer cannot modify core/api files", - username: "david", // david is developer + name: "Developer cannot modify core/api files", + role: "developer", files: []string{ "src/components/Button.js", "src/core/engine.js", @@ -57,8 +57,8 @@ func TestComplexRBACPatterns(t *testing.T) { }, }, { - name: "Viewer cannot modify any files", - username: "frank", // frank is viewer + name: "Viewer cannot modify any files", + role: "viewer", files: []string{ "src/components/Button.js", "README.md", @@ -77,9 +77,9 @@ func TestComplexRBACPatterns(t *testing.T) { // For now, we'll skip it as it needs the full integration setup t.Skip("Requires .sym/roles.json and .sym/user-policy.json setup") - result, err := roles.ValidateFilePermissions(tt.username, tt.files) + result, err := roles.ValidateFilePermissionsForRole(tt.role, tt.files) if err != nil { - t.Fatalf("ValidateFilePermissions failed: %v", err) + t.Fatalf("ValidateFilePermissionsForRole failed: %v", err) } if result.Allowed != tt.expectAllow { diff --git a/tests/integration/tsc_test.go b/tests/integration/tsc_test.go index a2b95dc..28629e7 100644 --- a/tests/integration/tsc_test.go +++ b/tests/integration/tsc_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/linter" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -54,7 +55,7 @@ func TestTSC_ValidateChanges(t *testing.T) { testFile := filepath.Join(testdataDir, "Test.ts") require.FileExists(t, testFile, "Test.ts should exist") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -124,7 +125,7 @@ func TestTSC_StrictNullChecks(t *testing.T) { assert.Contains(t, string(configData), `"strictNullChecks": true`, "Config should have strictNullChecks enabled") testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -176,7 +177,7 @@ func TestTSC_TypeErrors(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "", @@ -229,7 +230,7 @@ func TestTSC_ToolNameAndRuleID(t *testing.T) { } testFile := filepath.Join(testdataDir, "Test.ts") - changes := []validator.GitChange{{ + changes := []git.Change{{ FilePath: testFile, Status: "M", Diff: "",