From ddad910ac1426e21bfb34bc9b3384aefb75d50de Mon Sep 17 00:00:00 2001 From: hidetzu Date: Wed, 1 Apr 2026 07:14:26 +0900 Subject: [PATCH] Implement Step2: practical level with hotspot, test detection, and per-file risk - Git log integration for hotspot detection (90 days, 5+ changes) - S-4: hotspot signal (file-scoped) - S-5: no_test_change signal (aggregate, multi-language test patterns) - S-6: core_module signal (config, payment, database, etc.) - S-7: security_module signal (auth, crypto, token, etc.) - Analyzer dedup: security_module takes priority over core_module - Per-file risk scoring (file-scoped signal weights / 40.0) - JSON/text output includes files[] with per-file risk and signals - Exit code: 0 (low), 1 (medium+), 2 (error) - Update spec.md to match actual git.Log signature - 50 unit tests across all packages Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 25 +++-- cmd/root.go | 20 +++- internal/analyzer/analyzer.go | 26 ++++- internal/analyzer/analyzer_test.go | 84 ++++++++++++++- internal/formatter/formatter_test.go | 69 ++++++++++-- internal/formatter/json.go | 10 +- internal/formatter/text.go | 16 +++ internal/git/diff_test.go | 43 ++++++++ internal/git/git.go | 34 ++++++ internal/scorer/scorer.go | 63 ++++++++++- internal/scorer/scorer_test.go | 77 ++++++++++++++ internal/signal/core_module.go | 65 ++++++++++++ internal/signal/core_module_test.go | 55 ++++++++++ internal/signal/hotspot.go | 59 +++++++++++ internal/signal/hotspot_test.go | 134 ++++++++++++++++++++++++ internal/signal/no_test_change.go | 114 ++++++++++++++++++++ internal/signal/no_test_change_test.go | 117 +++++++++++++++++++++ internal/signal/security_module.go | 65 ++++++++++++ internal/signal/security_module_test.go | 55 ++++++++++ main.go | 6 +- specs/spec.md | 5 +- 21 files changed, 1110 insertions(+), 32 deletions(-) create mode 100644 internal/signal/core_module.go create mode 100644 internal/signal/core_module_test.go create mode 100644 internal/signal/hotspot.go create mode 100644 internal/signal/hotspot_test.go create mode 100644 internal/signal/no_test_change.go create mode 100644 internal/signal/no_test_change_test.go create mode 100644 internal/signal/security_module.go create mode 100644 internal/signal/security_module_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 15a2567..936ca98 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,23 +50,26 @@ Temporary, task-scoped specs derived from the master specs. ## Current Focus -**Step1: Minimal Working Version** (see `specs/roadmap.md`) +**Step2: Practical Level** (see `specs/roadmap.md`) ### Tasks -- [x] Project scaffold (`go mod init`, cobra CLI, main.go) -- [x] Git diff parsing (`git diff --numstat`) -- [x] S-1: large_change signal -- [x] S-2: high_insertions signal -- [x] S-3: high_deletions signal -- [x] Scorer (sum weights, clip 0-100, assign level) -- [x] JSON formatter -- [x] Text formatter +- [x] Git log integration (`git log --name-only`) +- [x] S-4: hotspot signal (90 days, 5+ changes) +- [x] S-5: no_test_change signal (aggregate) +- [x] S-6: core_module signal (file-scoped) +- [x] S-7: security_module signal (file-scoped, priority over core) +- [x] Analyzer priority dedup (security > core) +- [x] Scorer per-file risk (file-scoped weights / 40.0) +- [x] Formatter update (files[] in JSON/text) +- [x] CLI exit code (non-zero for medium+) ### Exit Criteria -- `riskcheck --base origin/main` outputs valid JSON with score, level, summary, and reasons -- `riskcheck --base origin/main --format text` outputs readable summary +- Hotspot files detected from git history +- Missing test changes flagged +- JSON output includes `files[]` with per-file risk and signals +- Exit code reflects risk level ## Tech Stack diff --git a/cmd/root.go b/cmd/root.go index 1c584e0..d0bfa6c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -13,6 +13,9 @@ import ( "github.com/spf13/cobra" ) +// ErrRiskThresholdExceeded is returned when the risk score is medium or above. +var ErrRiskThresholdExceeded = fmt.Errorf("risk threshold exceeded") + var ( version = "dev" @@ -33,6 +36,8 @@ func init() { rootCmd.Flags().StringVar(&flagBase, "base", "origin/main", "Comparison base") rootCmd.Flags().StringVar(&flagTarget, "target", ".", "Comparison target (working tree)") rootCmd.Flags().StringVar(&flagFormat, "format", "json", "Output format: json, text") + rootCmd.SilenceErrors = true + rootCmd.SilenceUsage = true } func Execute() error { @@ -54,6 +59,10 @@ func run(cmd *cobra.Command, args []string) error { signal.NewLargeChange(), signal.NewHighInsertions(), signal.NewHighDeletions(), + signal.NewHotspot(gc), + signal.NewNoTestChange(), + signal.NewSecurityModule(), + signal.NewCoreModule(), ) signals, err := a.Analyze(ctx, diff) if err != nil { @@ -80,5 +89,14 @@ func run(cmd *cobra.Command, args []string) error { } _, err = fmt.Fprintln(os.Stdout, string(out)) - return err + if err != nil { + return err + } + + // Non-zero exit for medium+ risk + if result.Score >= 40 { + return ErrRiskThresholdExceeded + } + + return nil } diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 4058ec5..4f37703 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -18,6 +18,7 @@ func New(signals ...signal.Signal) *Analyzer { } // Analyze runs all signals and collects detected signals. +// It applies priority rules: security_module > core_module (no double counting). func (a *Analyzer) Analyze(ctx context.Context, diff *git.DiffResult) ([]signal.DetectedSignal, error) { var result []signal.DetectedSignal @@ -29,5 +30,28 @@ func (a *Analyzer) Analyze(ctx context.Context, diff *git.DiffResult) ([]signal. result = append(result, detected...) } - return result, nil + return dedup(result), nil +} + +// dedup removes core_module signals for files that also have security_module signals. +func dedup(signals []signal.DetectedSignal) []signal.DetectedSignal { + securityFiles := make(map[string]bool) + for _, s := range signals { + if s.SignalName == "security_module" && s.FilePath != "" { + securityFiles[s.FilePath] = true + } + } + + if len(securityFiles) == 0 { + return signals + } + + var result []signal.DetectedSignal + for _, s := range signals { + if s.SignalName == "core_module" && securityFiles[s.FilePath] { + continue + } + result = append(result, s) + } + return result } diff --git a/internal/analyzer/analyzer_test.go b/internal/analyzer/analyzer_test.go index 05b30ce..955732e 100644 --- a/internal/analyzer/analyzer_test.go +++ b/internal/analyzer/analyzer_test.go @@ -28,7 +28,6 @@ func TestAnalyzer_MultipleSignals(t *testing.T) { signal.NewHighDeletions(), ) - // 15 files, 300 insertions each, 300 deletions each → all 3 signals fire files := make([]git.FileDiff, 15) for i := range files { files[i] = git.FileDiff{Path: "f.go", Insertions: 20, Deletions: 20} @@ -65,3 +64,86 @@ func TestAnalyzer_NoDetection(t *testing.T) { t.Errorf("got %d signals, want 0", len(result)) } } + +func TestAnalyzer_SecurityOverCore(t *testing.T) { + a := New( + signal.NewSecurityModule(), + signal.NewCoreModule(), + ) + + tests := []struct { + name string + path string + wantSignals int + wantNames []string + }{ + { + name: "auth matches security only (not core)", + path: "src/auth/login.go", + wantSignals: 1, + wantNames: []string{"security_module"}, + }, + { + name: "config matches core only", + path: "src/config/app.go", + wantSignals: 1, + wantNames: []string{"core_module"}, + }, + { + name: "unrelated matches neither", + path: "src/handler/home.go", + wantSignals: 0, + wantNames: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + diff := &git.DiffResult{ + Files: []git.FileDiff{{Path: tt.path}}, + } + result, err := a.Analyze(context.Background(), diff) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(result) != tt.wantSignals { + t.Fatalf("got %d signals, want %d", len(result), tt.wantSignals) + } + for i, name := range tt.wantNames { + if result[i].SignalName != name { + t.Errorf("signal[%d] = %q, want %q", i, result[i].SignalName, name) + } + } + }) + } +} + +func TestDedup_SecurityPriority(t *testing.T) { + signals := []signal.DetectedSignal{ + {SignalName: "security_module", FilePath: "src/auth/login.go", Weight: 20}, + {SignalName: "core_module", FilePath: "src/auth/login.go", Weight: 20}, + {SignalName: "core_module", FilePath: "src/config/app.go", Weight: 20}, + {SignalName: "large_change", FilePath: "", Weight: 10}, + } + + result := dedup(signals) + + if len(result) != 3 { + t.Fatalf("got %d signals, want 3", len(result)) + } + + names := make(map[string]int) + for _, s := range result { + names[s.SignalName]++ + } + + if names["security_module"] != 1 { + t.Errorf("security_module count = %d, want 1", names["security_module"]) + } + if names["core_module"] != 1 { + t.Errorf("core_module count = %d, want 1 (only config)", names["core_module"]) + } + if names["large_change"] != 1 { + t.Errorf("large_change count = %d, want 1", names["large_change"]) + } +} diff --git a/internal/formatter/formatter_test.go b/internal/formatter/formatter_test.go index 263a038..5396e6b 100644 --- a/internal/formatter/formatter_test.go +++ b/internal/formatter/formatter_test.go @@ -10,14 +10,18 @@ import ( func TestJSON_Format(t *testing.T) { result := &scorer.ScoreResult{ - Score: 25, - Level: "low", + Score: 45, + Level: "medium", Summary: scorer.Summary{ FilesChanged: 3, Insertions: 45, Deletions: 12, }, Reasons: []string{"high number of insertions (250 lines)"}, + Files: []scorer.FileRisk{ + {Path: "src/auth/login.go", Risk: 0.75, Signals: []string{"hotspot", "security_module"}}, + {Path: "src/main.go", Risk: 0, Signals: nil}, + }, } f := NewJSON() @@ -26,18 +30,21 @@ func TestJSON_Format(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - // Verify it's valid JSON var parsed map[string]interface{} if err := json.Unmarshal(out, &parsed); err != nil { t.Fatalf("invalid JSON: %v", err) } - // Verify required keys - for _, key := range []string{"score", "level", "summary", "reasons"} { + for _, key := range []string{"score", "level", "summary", "reasons", "files"} { if _, ok := parsed[key]; !ok { t.Errorf("missing key %q in JSON output", key) } } + + files := parsed["files"].([]interface{}) + if len(files) != 2 { + t.Errorf("files count = %d, want 2", len(files)) + } } func TestJSON_EmptyReasons(t *testing.T) { @@ -46,6 +53,33 @@ func TestJSON_EmptyReasons(t *testing.T) { Level: "low", Summary: scorer.Summary{}, Reasons: nil, + Files: nil, + } + + f := NewJSON() + out, err := f.Format(result) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + s := string(out) + if !strings.Contains(s, `"reasons": []`) { + t.Errorf("expected reasons to be [], got: %s", s) + } + if !strings.Contains(s, `"files": []`) { + t.Errorf("expected files to be [], got: %s", s) + } +} + +func TestJSON_NilSignalsInFile(t *testing.T) { + result := &scorer.ScoreResult{ + Score: 0, + Level: "low", + Summary: scorer.Summary{}, + Reasons: []string{}, + Files: []scorer.FileRisk{ + {Path: "a.go", Risk: 0, Signals: nil}, + }, } f := NewJSON() @@ -54,22 +88,25 @@ func TestJSON_EmptyReasons(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - // Verify reasons is [] not null - if !strings.Contains(string(out), `"reasons": []`) { - t.Errorf("expected reasons to be [], got: %s", out) + if strings.Contains(string(out), `"signals": null`) { + t.Error("signals should be [] not null") } } func TestText_Format(t *testing.T) { result := &scorer.ScoreResult{ - Score: 25, - Level: "low", + Score: 45, + Level: "medium", Summary: scorer.Summary{ FilesChanged: 3, Insertions: 45, Deletions: 12, }, Reasons: []string{"high number of insertions"}, + Files: []scorer.FileRisk{ + {Path: "src/auth/login.go", Risk: 0.75, Signals: []string{"hotspot", "security_module"}}, + {Path: "src/main.go", Risk: 0, Signals: nil}, + }, } f := NewText() @@ -81,12 +118,14 @@ func TestText_Format(t *testing.T) { text := string(out) checks := []string{ - "Risk Score: 25 / 100 (low)", + "Risk Score: 45 / 100 (medium)", "Files changed: 3", "Insertions: 45", "Deletions: 12", "Reasons:", "- high number of insertions", + "High-risk files:", + "0.75 src/auth/login.go [hotspot, security_module]", } for _, c := range checks { @@ -94,6 +133,11 @@ func TestText_Format(t *testing.T) { t.Errorf("output missing %q\ngot:\n%s", c, text) } } + + // src/main.go should NOT appear (risk = 0) + if strings.Contains(text, "src/main.go") { + t.Error("should not show files with zero risk") + } } func TestText_NoReasons(t *testing.T) { @@ -112,4 +156,7 @@ func TestText_NoReasons(t *testing.T) { if strings.Contains(string(out), "Reasons:") { t.Error("should not contain Reasons section when empty") } + if strings.Contains(string(out), "High-risk files:") { + t.Error("should not contain High-risk files section when empty") + } } diff --git a/internal/formatter/json.go b/internal/formatter/json.go index 2964b92..39f8d72 100644 --- a/internal/formatter/json.go +++ b/internal/formatter/json.go @@ -14,9 +14,17 @@ func NewJSON() *JSON { } func (f *JSON) Format(result *scorer.ScoreResult) ([]byte, error) { - // Ensure reasons is always an array, not null + // Ensure slices are always arrays, not null if result.Reasons == nil { result.Reasons = []string{} } + if result.Files == nil { + result.Files = []scorer.FileRisk{} + } + for i := range result.Files { + if result.Files[i].Signals == nil { + result.Files[i].Signals = []string{} + } + } return json.MarshalIndent(result, "", " ") } diff --git a/internal/formatter/text.go b/internal/formatter/text.go index 39caa26..ff1f80d 100644 --- a/internal/formatter/text.go +++ b/internal/formatter/text.go @@ -31,5 +31,21 @@ func (f *Text) Format(result *scorer.ScoreResult) ([]byte, error) { } } + // Show files with non-zero risk + var riskyFiles []scorer.FileRisk + for _, f := range result.Files { + if f.Risk > 0 { + riskyFiles = append(riskyFiles, f) + } + } + + if len(riskyFiles) > 0 { + fmt.Fprintln(&b) + fmt.Fprintln(&b, "High-risk files:") + for _, f := range riskyFiles { + fmt.Fprintf(&b, " %.2f %s [%s]\n", f.Risk, f.Path, strings.Join(f.Signals, ", ")) + } + } + return []byte(b.String()), nil } diff --git a/internal/git/diff_test.go b/internal/git/diff_test.go index c88d0d9..0f81ba1 100644 --- a/internal/git/diff_test.go +++ b/internal/git/diff_test.go @@ -92,6 +92,49 @@ func TestParseNumstat(t *testing.T) { } } +func TestParseNameOnly(t *testing.T) { + tests := []struct { + name string + input string + want FileChangeCount + }{ + { + name: "empty input", + input: "", + want: FileChangeCount{}, + }, + { + name: "single file single commit", + input: "src/main.go", + want: FileChangeCount{"src/main.go": 1}, + }, + { + name: "same file multiple commits", + input: "src/main.go\n\nsrc/main.go\n\nsrc/util.go", + want: FileChangeCount{"src/main.go": 2, "src/util.go": 1}, + }, + { + name: "multiple files with blank lines", + input: "src/a.go\nsrc/b.go\n\nsrc/a.go\nsrc/c.go\n\n", + want: FileChangeCount{"src/a.go": 2, "src/b.go": 1, "src/c.go": 1}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseNameOnly(tt.input) + if len(got) != len(tt.want) { + t.Fatalf("got %d entries, want %d", len(got), len(tt.want)) + } + for k, v := range tt.want { + if got[k] != v { + t.Errorf("file %q: count = %d, want %d", k, got[k], v) + } + } + }) + } +} + func TestDiffResultTotals(t *testing.T) { d := &DiffResult{ Files: []FileDiff{ diff --git a/internal/git/git.go b/internal/git/git.go index 927ff17..6fb2134 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -59,3 +59,37 @@ func (c *Client) Diff(ctx context.Context, base, target string) (*DiffResult, er return ParseNumstat(strings.TrimSpace(string(out))) } + +// FileChangeCount maps file paths to their change frequency. +type FileChangeCount map[string]int + +// Log runs git log --name-only and returns file change counts. +func (c *Client) Log(ctx context.Context, since string) (FileChangeCount, error) { + args := []string{"log", "--since", since, "--name-only", "--format="} + + cmd := exec.CommandContext(ctx, "git", args...) + out, err := cmd.Output() + if err != nil { + return nil, err + } + + return ParseNameOnly(strings.TrimSpace(string(out))), nil +} + +// ParseNameOnly parses git log --name-only output into file change counts. +func ParseNameOnly(output string) FileChangeCount { + counts := make(FileChangeCount) + if output == "" { + return counts + } + + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + counts[line]++ + } + + return counts +} diff --git a/internal/scorer/scorer.go b/internal/scorer/scorer.go index 3df657d..98cd718 100644 --- a/internal/scorer/scorer.go +++ b/internal/scorer/scorer.go @@ -1,16 +1,22 @@ package scorer import ( + "math" + "sort" + "github.com/hidetzu/riskcheck/internal/git" "github.com/hidetzu/riskcheck/internal/signal" ) +const perFileNormalizationDivisor = 40.0 + // ScoreResult holds the final risk assessment. type ScoreResult struct { - Score int `json:"score"` - Level string `json:"level"` - Summary Summary `json:"summary"` - Reasons []string `json:"reasons"` + Score int `json:"score"` + Level string `json:"level"` + Summary Summary `json:"summary"` + Reasons []string `json:"reasons"` + Files []FileRisk `json:"files"` } // Summary holds aggregate change statistics. @@ -20,6 +26,13 @@ type Summary struct { Deletions int `json:"deletions"` } +// FileRisk holds per-file risk information. +type FileRisk struct { + Path string `json:"path"` + Risk float64 `json:"risk"` + Signals []string `json:"signals"` +} + // Score calculates the risk score from detected signals and diff data. func Score(signals []signal.DetectedSignal, diff *git.DiffResult) *ScoreResult { total := 0 @@ -43,6 +56,7 @@ func Score(signals []signal.DetectedSignal, diff *git.DiffResult) *ScoreResult { Level: level(total), Summary: makeSummary(diff), Reasons: reasons, + Files: calcFileRisks(signals, diff), } } @@ -64,3 +78,44 @@ func makeSummary(diff *git.DiffResult) Summary { Deletions: diff.TotalDeletions(), } } + +// calcFileRisks calculates per-file risk from file-scoped signals only. +// Aggregate signals (empty FilePath) are excluded. +func calcFileRisks(signals []signal.DetectedSignal, diff *git.DiffResult) []FileRisk { + // Collect file-scoped signals grouped by path + fileSignals := make(map[string][]signal.DetectedSignal) + for _, s := range signals { + if s.FilePath != "" { + fileSignals[s.FilePath] = append(fileSignals[s.FilePath], s) + } + } + + // Build FileRisk for all changed files + var risks []FileRisk + for _, f := range diff.Files { + sigs := fileSignals[f.Path] + weightSum := 0 + sigNames := []string{} + for _, s := range sigs { + weightSum += s.Weight + sigNames = append(sigNames, s.SignalName) + } + + risk := math.Min(float64(weightSum)/perFileNormalizationDivisor, 1.0) + // Round to 2 decimal places + risk = math.Round(risk*100) / 100 + + risks = append(risks, FileRisk{ + Path: f.Path, + Risk: risk, + Signals: sigNames, + }) + } + + // Sort by risk descending + sort.Slice(risks, func(i, j int) bool { + return risks[i].Risk > risks[j].Risk + }) + + return risks +} diff --git a/internal/scorer/scorer_test.go b/internal/scorer/scorer_test.go index 8c2a0a3..d2f09af 100644 --- a/internal/scorer/scorer_test.go +++ b/internal/scorer/scorer_test.go @@ -28,6 +28,13 @@ func TestScore_NoSignals(t *testing.T) { if result.Summary.FilesChanged != 1 { t.Errorf("FilesChanged = %d, want 1", result.Summary.FilesChanged) } + // Per-file risk should be 0 for file with no signals + if len(result.Files) != 1 { + t.Fatalf("Files count = %d, want 1", len(result.Files)) + } + if result.Files[0].Risk != 0 { + t.Errorf("File risk = %f, want 0", result.Files[0].Risk) + } } func TestScore_SingleSignal(t *testing.T) { @@ -108,3 +115,73 @@ func TestScore_LevelBoundaries(t *testing.T) { } } } + +func TestScore_PerFileRisk(t *testing.T) { + signals := []signal.DetectedSignal{ + // File-scoped signals + {SignalName: "hotspot", FilePath: "src/auth/login.go", Weight: 10, Reason: "r1"}, + {SignalName: "security_module", FilePath: "src/auth/login.go", Weight: 20, Reason: "r2"}, + {SignalName: "core_module", FilePath: "src/config/app.go", Weight: 20, Reason: "r3"}, + // Aggregate signal — should NOT affect per-file risk + {SignalName: "no_test_change", FilePath: "", Weight: 15, Reason: "r4"}, + } + + diff := &git.DiffResult{ + Files: []git.FileDiff{ + {Path: "src/auth/login.go", Insertions: 10, Deletions: 5}, + {Path: "src/config/app.go", Insertions: 3, Deletions: 1}, + {Path: "src/handler/home.go", Insertions: 20, Deletions: 10}, + }, + } + + result := Score(signals, diff) + + if len(result.Files) != 3 { + t.Fatalf("Files count = %d, want 3", len(result.Files)) + } + + // Sorted by risk descending + // auth/login.go: (10+20)/40 = 0.75 + if result.Files[0].Path != "src/auth/login.go" { + t.Errorf("Files[0].Path = %q, want src/auth/login.go", result.Files[0].Path) + } + if result.Files[0].Risk != 0.75 { + t.Errorf("Files[0].Risk = %f, want 0.75", result.Files[0].Risk) + } + if len(result.Files[0].Signals) != 2 { + t.Errorf("Files[0].Signals count = %d, want 2", len(result.Files[0].Signals)) + } + + // config/app.go: 20/40 = 0.50 + if result.Files[1].Path != "src/config/app.go" { + t.Errorf("Files[1].Path = %q, want src/config/app.go", result.Files[1].Path) + } + if result.Files[1].Risk != 0.5 { + t.Errorf("Files[1].Risk = %f, want 0.5", result.Files[1].Risk) + } + + // handler/home.go: no file-scoped signals → 0.0 + if result.Files[2].Path != "src/handler/home.go" { + t.Errorf("Files[2].Path = %q, want src/handler/home.go", result.Files[2].Path) + } + if result.Files[2].Risk != 0 { + t.Errorf("Files[2].Risk = %f, want 0", result.Files[2].Risk) + } +} + +func TestScore_PerFileRiskClip(t *testing.T) { + // Weight sum exceeds 40 → clipped to 1.0 + signals := []signal.DetectedSignal{ + {SignalName: "security_module", FilePath: "a.go", Weight: 20}, + {SignalName: "hotspot", FilePath: "a.go", Weight: 10}, + {SignalName: "core_module", FilePath: "a.go", Weight: 20}, + } + diff := &git.DiffResult{ + Files: []git.FileDiff{{Path: "a.go"}}, + } + + result := Score(signals, diff) + if result.Files[0].Risk != 1.0 { + t.Errorf("Risk = %f, want 1.0 (clipped)", result.Files[0].Risk) + } +} diff --git a/internal/signal/core_module.go b/internal/signal/core_module.go new file mode 100644 index 0000000..275c757 --- /dev/null +++ b/internal/signal/core_module.go @@ -0,0 +1,65 @@ +package signal + +import ( + "context" + "fmt" + "strings" + + "github.com/hidetzu/riskcheck/internal/git" +) + +const ( + CoreModuleDefaultWeight = 20 +) + +// DefaultCorePaths defines paths considered core/critical. +var DefaultCorePaths = []string{ + "config/", + "billing/", + "payment/", + "database/", + "migration/", + "infra/", + "api/", +} + +// CoreModule detects changes to core business logic paths. +type CoreModule struct { + Weight int + Paths []string +} + +func NewCoreModule() *CoreModule { + return &CoreModule{ + Weight: CoreModuleDefaultWeight, + Paths: DefaultCorePaths, + } +} + +func (s *CoreModule) Name() string { + return "core_module" +} + +func (s *CoreModule) Detect(_ context.Context, diff *git.DiffResult) ([]DetectedSignal, error) { + var signals []DetectedSignal + for _, f := range diff.Files { + if s.matchesCorePath(f.Path) { + signals = append(signals, DetectedSignal{ + SignalName: s.Name(), + FilePath: f.Path, + Weight: s.Weight, + Reason: fmt.Sprintf("core module modified (%s)", f.Path), + }) + } + } + return signals, nil +} + +func (s *CoreModule) matchesCorePath(path string) bool { + for _, p := range s.Paths { + if strings.Contains(path, p) { + return true + } + } + return false +} diff --git a/internal/signal/core_module_test.go b/internal/signal/core_module_test.go new file mode 100644 index 0000000..669a1f9 --- /dev/null +++ b/internal/signal/core_module_test.go @@ -0,0 +1,55 @@ +package signal + +import ( + "context" + "testing" + + "github.com/hidetzu/riskcheck/internal/git" +) + +func TestCoreModule(t *testing.T) { + s := NewCoreModule() + + if s.Name() != "core_module" { + t.Errorf("Name() = %q, want %q", s.Name(), "core_module") + } + + tests := []struct { + name string + path string + detected bool + }{ + {"config path", "src/config/app.go", true}, + {"billing path", "src/billing/invoice.go", true}, + {"payment path", "src/payment/stripe.go", true}, + {"database path", "internal/database/conn.go", true}, + {"migration path", "db/migration/001_init.sql", true}, + {"infra path", "infra/terraform/main.tf", true}, + {"api path", "src/api/handler.go", true}, + {"non-core path", "src/handler/home.go", false}, + {"auth path (not core)", "src/auth/login.go", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + diff := &git.DiffResult{ + Files: []git.FileDiff{{Path: tt.path}}, + } + signals, err := s.Detect(context.Background(), diff) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := len(signals) > 0; got != tt.detected { + t.Errorf("detected = %v, want %v", got, tt.detected) + } + if tt.detected { + if signals[0].Weight != CoreModuleDefaultWeight { + t.Errorf("Weight = %d, want %d", signals[0].Weight, CoreModuleDefaultWeight) + } + if signals[0].FilePath != tt.path { + t.Errorf("FilePath = %q, want %q", signals[0].FilePath, tt.path) + } + } + }) + } +} diff --git a/internal/signal/hotspot.go b/internal/signal/hotspot.go new file mode 100644 index 0000000..7042a09 --- /dev/null +++ b/internal/signal/hotspot.go @@ -0,0 +1,59 @@ +package signal + +import ( + "context" + "fmt" + + "github.com/hidetzu/riskcheck/internal/git" +) + +const ( + HotspotDefaultSince = "90 days ago" + HotspotDefaultThreshold = 5 + HotspotDefaultWeight = 10 +) + +// Hotspot detects files that have been frequently changed in recent history. +type Hotspot struct { + GitClient *git.Client + Since string + Threshold int + Weight int +} + +func NewHotspot(gitClient *git.Client) *Hotspot { + return &Hotspot{ + GitClient: gitClient, + Since: HotspotDefaultSince, + Threshold: HotspotDefaultThreshold, + Weight: HotspotDefaultWeight, + } +} + +func (s *Hotspot) Name() string { + return "hotspot" +} + +func (s *Hotspot) Detect(ctx context.Context, diff *git.DiffResult) ([]DetectedSignal, error) { + counts, err := s.GitClient.Log(ctx, s.Since) + if err != nil { + return nil, fmt.Errorf("git log: %w", err) + } + + return s.detect(diff, counts), nil +} + +func (s *Hotspot) detect(diff *git.DiffResult, counts git.FileChangeCount) []DetectedSignal { + var signals []DetectedSignal + for _, f := range diff.Files { + if counts[f.Path] >= s.Threshold { + signals = append(signals, DetectedSignal{ + SignalName: s.Name(), + FilePath: f.Path, + Weight: s.Weight, + Reason: fmt.Sprintf("hotspot file touched (%s changed %d times in last %s)", f.Path, counts[f.Path], s.Since), + }) + } + } + return signals +} diff --git a/internal/signal/hotspot_test.go b/internal/signal/hotspot_test.go new file mode 100644 index 0000000..d5e6560 --- /dev/null +++ b/internal/signal/hotspot_test.go @@ -0,0 +1,134 @@ +package signal + +import ( + "context" + "strings" + "testing" + + "github.com/hidetzu/riskcheck/internal/git" +) + +func TestHotspot(t *testing.T) { + s := &Hotspot{ + Since: HotspotDefaultSince, + Threshold: HotspotDefaultThreshold, + Weight: HotspotDefaultWeight, + } + + if s.Name() != "hotspot" { + t.Errorf("Name() = %q, want %q", s.Name(), "hotspot") + } + + tests := []struct { + name string + diff *git.DiffResult + counts git.FileChangeCount + detected int + }{ + { + name: "file above threshold", + diff: &git.DiffResult{ + Files: []git.FileDiff{{Path: "src/main.go"}}, + }, + counts: git.FileChangeCount{"src/main.go": 7}, + detected: 1, + }, + { + name: "file below threshold", + diff: &git.DiffResult{ + Files: []git.FileDiff{{Path: "src/main.go"}}, + }, + counts: git.FileChangeCount{"src/main.go": 3}, + detected: 0, + }, + { + name: "file at threshold", + diff: &git.DiffResult{ + Files: []git.FileDiff{{Path: "src/main.go"}}, + }, + counts: git.FileChangeCount{"src/main.go": 5}, + detected: 1, + }, + { + name: "file not in log", + diff: &git.DiffResult{ + Files: []git.FileDiff{{Path: "src/new.go"}}, + }, + counts: git.FileChangeCount{}, + detected: 0, + }, + { + name: "multiple hotspot files", + diff: &git.DiffResult{ + Files: []git.FileDiff{ + {Path: "src/a.go"}, + {Path: "src/b.go"}, + {Path: "src/c.go"}, + }, + }, + counts: git.FileChangeCount{ + "src/a.go": 10, + "src/b.go": 2, + "src/c.go": 6, + }, + detected: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + signals := s.detect(tt.diff, tt.counts) + if len(signals) != tt.detected { + t.Errorf("got %d signals, want %d", len(signals), tt.detected) + } + for _, sig := range signals { + if sig.Weight != HotspotDefaultWeight { + t.Errorf("Weight = %d, want %d", sig.Weight, HotspotDefaultWeight) + } + if sig.FilePath == "" { + t.Error("FilePath should not be empty for file-scoped signal") + } + } + }) + } +} + +func TestHotspot_Detect_Integration(t *testing.T) { + // Test the public Detect() method with a real git client on this repo + gc := git.NewClient() + s := NewHotspot(gc) + + diff := &git.DiffResult{ + Files: []git.FileDiff{{Path: "nonexistent_file.go"}}, + } + + signals, err := s.Detect(context.Background(), diff) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // nonexistent file should not be a hotspot + if len(signals) != 0 { + t.Errorf("got %d signals, want 0", len(signals)) + } +} + +func TestHotspot_ReasonUsesSinceValue(t *testing.T) { + s := &Hotspot{ + Since: "30 days ago", + Threshold: 1, + Weight: 10, + } + + diff := &git.DiffResult{ + Files: []git.FileDiff{{Path: "a.go"}}, + } + counts := git.FileChangeCount{"a.go": 5} + + signals := s.detect(diff, counts) + if len(signals) != 1 { + t.Fatalf("got %d signals, want 1", len(signals)) + } + if !strings.Contains(signals[0].Reason, "30 days ago") { + t.Errorf("reason should contain since value, got: %s", signals[0].Reason) + } +} diff --git a/internal/signal/no_test_change.go b/internal/signal/no_test_change.go new file mode 100644 index 0000000..5515e55 --- /dev/null +++ b/internal/signal/no_test_change.go @@ -0,0 +1,114 @@ +package signal + +import ( + "context" + "path/filepath" + "strings" + + "github.com/hidetzu/riskcheck/internal/git" +) + +const ( + NoTestChangeDefaultWeight = 15 +) + +// DefaultTestPatterns defines patterns for identifying test files across languages. +var DefaultTestPatterns = []string{ + // Go + "*_test.go", + // Python + "test_*.py", + "*_test.py", + // TypeScript / JavaScript + "*.test.ts", + "*.spec.ts", + "*.test.tsx", + "*.spec.tsx", + "*.test.js", + "*.spec.js", + "*.test.jsx", + "*.spec.jsx", + // Java / Kotlin + "*Test.java", + "*Tests.java", + "*Test.kt", + "*Tests.kt", + // C# + "*Test.cs", + "*Tests.cs", +} + +// DefaultTestDirPatterns defines directory patterns for test files. +var DefaultTestDirPatterns = []string{ + "tests/", +} + +// NoTestChange detects when production files are changed but no test files are updated. +type NoTestChange struct { + Weight int + TestPatterns []string + TestDirs []string +} + +func NewNoTestChange() *NoTestChange { + return &NoTestChange{ + Weight: NoTestChangeDefaultWeight, + TestPatterns: DefaultTestPatterns, + TestDirs: DefaultTestDirPatterns, + } +} + +func (s *NoTestChange) Name() string { + return "no_test_change" +} + +func (s *NoTestChange) Detect(_ context.Context, diff *git.DiffResult) ([]DetectedSignal, error) { + if len(diff.Files) == 0 { + return nil, nil + } + + hasProductionFile := false + hasTestFile := false + + for _, f := range diff.Files { + if s.isTestFile(f.Path) { + hasTestFile = true + } else { + hasProductionFile = true + } + } + + if hasProductionFile && !hasTestFile { + return []DetectedSignal{ + { + SignalName: s.Name(), + Weight: s.Weight, + Reason: "no test updates for changed files", + }, + }, nil + } + + return nil, nil +} + +func (s *NoTestChange) isTestFile(path string) bool { + base := filepath.Base(path) + + for _, pattern := range s.TestPatterns { + matched, err := filepath.Match(pattern, base) + if err != nil { + continue // skip invalid patterns + } + if matched { + return true + } + } + + for _, dir := range s.TestDirs { + if strings.HasPrefix(path, dir) { + return true + } + } + + return false +} diff --git a/internal/signal/no_test_change_test.go b/internal/signal/no_test_change_test.go new file mode 100644 index 0000000..433bb71 --- /dev/null +++ b/internal/signal/no_test_change_test.go @@ -0,0 +1,117 @@ +package signal + +import ( + "context" + "testing" + + "github.com/hidetzu/riskcheck/internal/git" +) + +func TestNoTestChange(t *testing.T) { + s := NewNoTestChange() + + if s.Name() != "no_test_change" { + t.Errorf("Name() = %q, want %q", s.Name(), "no_test_change") + } + + tests := []struct { + name string + files []string + detected bool + }{ + { + name: "production only - no tests", + files: []string{"src/main.go", "src/util.go"}, + detected: true, + }, + { + name: "production with Go test", + files: []string{"src/main.go", "src/main_test.go"}, + detected: false, + }, + { + name: "production with TS test", + files: []string{"src/app.ts", "src/app.test.ts"}, + detected: false, + }, + { + name: "production with spec.ts", + files: []string{"src/app.ts", "src/app.spec.ts"}, + detected: false, + }, + { + name: "production with JS test", + files: []string{"src/app.js", "src/app.test.js"}, + detected: false, + }, + { + name: "production with spec.jsx", + files: []string{"src/app.jsx", "src/app.spec.jsx"}, + detected: false, + }, + { + name: "production with Java test", + files: []string{"src/App.java", "src/AppTest.java"}, + detected: false, + }, + { + name: "production with Java Tests", + files: []string{"src/App.java", "src/AppTests.java"}, + detected: false, + }, + { + name: "production with Python test_", + files: []string{"src/app.py", "tests/test_app.py"}, + detected: false, + }, + { + name: "production with Python _test", + files: []string{"src/app.py", "tests/app_test.py"}, + detected: false, + }, + { + name: "production with C# test", + files: []string{"src/App.cs", "src/AppTest.cs"}, + detected: false, + }, + { + name: "production with Kotlin test", + files: []string{"src/App.kt", "src/AppTest.kt"}, + detected: false, + }, + { + name: "test files only", + files: []string{"src/main_test.go"}, + detected: false, + }, + { + name: "files in tests/ directory", + files: []string{"src/main.go", "tests/integration/test_main.py"}, + detected: false, + }, + { + name: "empty diff", + files: []string{}, + detected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + diff := &git.DiffResult{} + for _, f := range tt.files { + diff.Files = append(diff.Files, git.FileDiff{Path: f}) + } + signals, err := s.Detect(context.Background(), diff) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := len(signals) > 0; got != tt.detected { + t.Errorf("detected = %v, want %v", got, tt.detected) + } + if tt.detected && signals[0].Weight != NoTestChangeDefaultWeight { + t.Errorf("Weight = %d, want %d", signals[0].Weight, NoTestChangeDefaultWeight) + } + }) + } +} diff --git a/internal/signal/security_module.go b/internal/signal/security_module.go new file mode 100644 index 0000000..34ff320 --- /dev/null +++ b/internal/signal/security_module.go @@ -0,0 +1,65 @@ +package signal + +import ( + "context" + "fmt" + "strings" + + "github.com/hidetzu/riskcheck/internal/git" +) + +const ( + SecurityModuleDefaultWeight = 20 +) + +// DefaultSecurityPaths defines paths considered security-critical. +var DefaultSecurityPaths = []string{ + "auth/", + "security/", + "crypto/", + "permission/", + "acl/", + "token/", + "jwt/", +} + +// SecurityModule detects changes to security-related paths. +type SecurityModule struct { + Weight int + Paths []string +} + +func NewSecurityModule() *SecurityModule { + return &SecurityModule{ + Weight: SecurityModuleDefaultWeight, + Paths: DefaultSecurityPaths, + } +} + +func (s *SecurityModule) Name() string { + return "security_module" +} + +func (s *SecurityModule) Detect(_ context.Context, diff *git.DiffResult) ([]DetectedSignal, error) { + var signals []DetectedSignal + for _, f := range diff.Files { + if s.matchesSecurityPath(f.Path) { + signals = append(signals, DetectedSignal{ + SignalName: s.Name(), + FilePath: f.Path, + Weight: s.Weight, + Reason: fmt.Sprintf("security module modified (%s)", f.Path), + }) + } + } + return signals, nil +} + +func (s *SecurityModule) matchesSecurityPath(path string) bool { + for _, p := range s.Paths { + if strings.Contains(path, p) { + return true + } + } + return false +} diff --git a/internal/signal/security_module_test.go b/internal/signal/security_module_test.go new file mode 100644 index 0000000..698302b --- /dev/null +++ b/internal/signal/security_module_test.go @@ -0,0 +1,55 @@ +package signal + +import ( + "context" + "testing" + + "github.com/hidetzu/riskcheck/internal/git" +) + +func TestSecurityModule(t *testing.T) { + s := NewSecurityModule() + + if s.Name() != "security_module" { + t.Errorf("Name() = %q, want %q", s.Name(), "security_module") + } + + tests := []struct { + name string + path string + detected bool + }{ + {"auth path", "src/auth/login.go", true}, + {"security path", "internal/security/validate.go", true}, + {"crypto path", "pkg/crypto/hash.go", true}, + {"permission path", "src/permission/check.go", true}, + {"acl path", "src/acl/rules.go", true}, + {"token path", "src/token/generate.go", true}, + {"jwt path", "src/jwt/verify.go", true}, + {"non-security path", "src/handler/api.go", false}, + {"config path", "src/config/app.go", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + diff := &git.DiffResult{ + Files: []git.FileDiff{{Path: tt.path}}, + } + signals, err := s.Detect(context.Background(), diff) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := len(signals) > 0; got != tt.detected { + t.Errorf("detected = %v, want %v", got, tt.detected) + } + if tt.detected { + if signals[0].Weight != SecurityModuleDefaultWeight { + t.Errorf("Weight = %d, want %d", signals[0].Weight, SecurityModuleDefaultWeight) + } + if signals[0].FilePath != tt.path { + t.Errorf("FilePath = %q, want %q", signals[0].FilePath, tt.path) + } + } + }) + } +} diff --git a/main.go b/main.go index 0458275..3a5473c 100644 --- a/main.go +++ b/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "os" "github.com/hidetzu/riskcheck/cmd" @@ -8,6 +9,9 @@ import ( func main() { if err := cmd.Execute(); err != nil { - os.Exit(1) + if errors.Is(err, cmd.ErrRiskThresholdExceeded) { + os.Exit(1) + } + os.Exit(2) } } diff --git a/specs/spec.md b/specs/spec.md index 19c6812..ae7afbe 100644 --- a/specs/spec.md +++ b/specs/spec.md @@ -135,9 +135,12 @@ type FileDiff struct { Deletions int } +// FileChangeCount maps file paths to their change frequency. +type FileChangeCount map[string]int + type Client interface { Diff(ctx context.Context, base, target string) (*DiffResult, error) - Log(ctx context.Context, path string, limit int) ([]LogEntry, error) // Step2 + Log(ctx context.Context, since string) (FileChangeCount, error) // Step2 } ```