From e22df6ed000a05352c5e624130fb1c7bb2db7abc Mon Sep 17 00:00:00 2001 From: ODS Bot Date: Sat, 30 May 2026 08:00:02 +0300 Subject: [PATCH 1/3] feat: enterprise policy config, enhanced compliance report, and PR fix suggestions Three major enhancements for enterprise adoption and developer experience: 1. Enterprise .ods.yaml Policy Config (P1) - New internal/policy package with profile presets - Three built-in profiles: oss, enterprise, regulated - Configurable: branch types, PR sections, AI disclosure, severity levels - Merge strategy: profile preset < .ods.yaml < CLI flags - Policy-aware validation: ValidateBranchWithPolicy, etc. - --profile and --policy flags on root command 2. Enhanced ODS Compliance Report (P2) - Complete HTML template rewrite with score gauge, summary cards, fix suggestions section with copy buttons, responsive design - Markdown now includes policy info and fix suggestions - JSON includes policy_profile and fix_suggestions arrays - Enhanced SVG badge with policy prefix (ODS-OSS/ENT/REG) and N/100 score 3. PR Auto-fix Suggestions (P3) - New FixSuggestion struct (title, description, copy-paste template) - 14 distinct fix scenarios: branch, commit, and PR validation failures - Fix suggestions appear in CLI output, Markdown report, and PR comments - Templates are policy-aware (e.g., scope required only for enterprise/regulated) --- internal/cmd/report.go | 26 +- internal/cmd/root.go | 14 +- internal/cmd/validate.go | 39 ++- internal/policy/policy.go | 368 +++++++++++++++++++++ internal/policy/policy_test.go | 191 +++++++++++ internal/report/report.go | 567 ++++++++++++++++++++++++++------ internal/report/report_test.go | 10 +- internal/validator/validator.go | 281 ++++++++++++++-- 8 files changed, 1366 insertions(+), 130 deletions(-) create mode 100644 internal/policy/policy.go create mode 100644 internal/policy/policy_test.go diff --git a/internal/cmd/report.go b/internal/cmd/report.go index bb7d78a..816995c 100644 --- a/internal/cmd/report.go +++ b/internal/cmd/report.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" + "github.com/open-delivery-spec/cli/internal/policy" "github.com/open-delivery-spec/cli/internal/report" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -17,7 +18,9 @@ var reportCmd = &cobra.Command{ The command is convention-first: it reads GitHub Actions environment data when available and falls back to local git metadata. Missing PR-only context is -reported as skipped instead of requiring extra flags.`, +reported as skipped instead of requiring extra flags. + +Use --profile to select a compliance policy: oss, enterprise, or regulated.`, RunE: func(cmd *cobra.Command, args []string) error { outputDir := reportOutput if outputDir == "" { @@ -27,8 +30,24 @@ reported as skipped instead of requiring extra flags.`, outputDir = "ods-report" } + // Load policy from profile flag or config file + p, err := policy.LoadPolicy() + if err != nil { + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not load policy: %v (using defaults)\n", err) + } + inputs := report.DiscoverInputs() - result := report.Build(inputs, report.Options{Strict: strict, Check: viper.GetString("report.check")}) + result := report.Build(inputs, report.Options{ + Strict: strict, + Check: viper.GetString("report.check"), + }) + + // Ensure policy info is attached + if p != nil { + result.Policy = p + result.PolicyProfile = p.Profile + } + if err := report.WriteFiles(result, outputDir); err != nil { return fmt.Errorf("writing report: %w", err) } @@ -36,6 +55,9 @@ reported as skipped instead of requiring extra flags.`, fmt.Printf("ODS compliance report written to %s\n", outputDir) fmt.Printf("Status: %s\n", result.Status) fmt.Printf("Score: %d / 100\n", result.Score) + if p != nil { + fmt.Printf("Policy: %s\n", p.Profile) + } if result.Status == report.StatusNonCompliant { return fmt.Errorf("ODS compliance report is non-compliant") diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 801c1d1..d296390 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -11,10 +11,12 @@ import ( ) var ( - cfgFile string - specVer string - strict bool - schemaDir string + cfgFile string + specVer string + strict bool + schemaDir string + policyProfile string + policyFile string ) var rootCmd = &cobra.Command{ @@ -47,9 +49,13 @@ func init() { rootCmd.PersistentFlags().StringVar(&specVer, "spec-version", "1.0.0", "ODS spec version") rootCmd.PersistentFlags().BoolVar(&strict, "strict", false, "treat warnings as errors") rootCmd.PersistentFlags().StringVar(&schemaDir, "schema-dir", "", "custom schema directory path") + rootCmd.PersistentFlags().StringVar(&policyProfile, "profile", "", "compliance profile: oss, enterprise, regulated") + rootCmd.PersistentFlags().StringVar(&policyFile, "policy", "", "path to policy YAML file") viper.BindPFlag("spec_version", rootCmd.PersistentFlags().Lookup("spec-version")) viper.BindPFlag("strict", rootCmd.PersistentFlags().Lookup("strict")) + viper.BindPFlag("profile", rootCmd.PersistentFlags().Lookup("profile")) + viper.BindPFlag("policy_file", rootCmd.PersistentFlags().Lookup("policy")) } func initConfig() { diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index a1f3b05..594409d 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/open-delivery-spec/cli/internal/policy" "github.com/open-delivery-spec/cli/internal/validator" "github.com/spf13/cobra" ) @@ -26,11 +27,13 @@ var validateBranchCmd = &cobra.Command{ Examples: ods validate branch feature/add-oauth-login - ods validate branch bugfix/fix-null-pointer --strict`, + ods validate branch bugfix/fix-null-pointer --strict + ods validate branch feature/add-oauth --profile enterprise`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { name := args[0] - result, err := validator.ValidateBranch(name) + p, _ := policy.LoadPolicy() + result, err := validator.ValidateBranchWithPolicy(name, p) if err != nil { return err } @@ -45,13 +48,15 @@ var validateCommitCmd = &cobra.Command{ Examples: ods validate commit --file commit-msg.txt - git log -1 --format=%B | ods validate commit --stdin`, + git log -1 --format=%B | ods validate commit --stdin + ods validate commit --file msg.txt --profile regulated`, RunE: func(cmd *cobra.Command, args []string) error { msg, err := readInput() if err != nil { return fmt.Errorf("reading commit message: %w", err) } - result, err := validator.ValidateCommitMessage(msg) + p, _ := policy.LoadPolicy() + result, err := validator.ValidateCommitMessageWithPolicy(msg, p) if err != nil { return err } @@ -62,12 +67,18 @@ Examples: var validatePRCmd = &cobra.Command{ Use: "pr", Short: "Validate a PR description", + Long: `Validate a PR description against the ODS PR Description spec. + +Examples: + ods validate pr --file PR_BODY.md + ods validate pr --file body.md --profile enterprise`, RunE: func(cmd *cobra.Command, args []string) error { body, err := readInput() if err != nil { return fmt.Errorf("reading PR description: %w", err) } - result, err := validator.ValidatePRDescription(body) + p, _ := policy.LoadPolicy() + result, err := validator.ValidatePRDescriptionWithPolicy(body, p) if err != nil { return err } @@ -186,6 +197,7 @@ func printResult(r validator.Result) error { for _, w := range r.Warnings { fmt.Printf(" - %s\n", w) } + printFixSuggestions(r) if strict && len(r.Warnings) > 0 { return fmt.Errorf("validation failed with %d warning(s) in strict mode", len(r.Warnings)) } @@ -194,6 +206,7 @@ func printResult(r validator.Result) error { for _, e := range r.Errors { fmt.Printf(" - %s\n", e) } + printFixSuggestions(r) errCount := len(r.Errors) if strict && len(r.Warnings) > 0 { fmt.Println("\nWarnings (treated as errors in strict mode):") @@ -207,6 +220,22 @@ func printResult(r validator.Result) error { return nil } +func printFixSuggestions(r validator.Result) { + if len(r.FixSuggestions) == 0 { + return + } + fmt.Println("\nšŸ”§ Fix suggestions:") + for i, fs := range r.FixSuggestions { + fmt.Printf(" %d. %s\n", i+1, fs.Title) + if fs.Description != "" { + fmt.Printf(" %s\n", fs.Description) + } + if fs.Template != "" { + fmt.Printf(" Template: %s\n", fs.Template) + } + } +} + // ioReadAll avoids importing io/ioutil (deprecated) and io for minimal deps func ioReadAll(f *os.File) ([]byte, error) { var buf []byte diff --git a/internal/policy/policy.go b/internal/policy/policy.go new file mode 100644 index 0000000..a325eae --- /dev/null +++ b/internal/policy/policy.go @@ -0,0 +1,368 @@ +// Package policy provides enterprise-grade ODS policy configuration. +// It supports configurable branch types, PR sections, AI disclosure rules, +// severity levels, and profile presets (oss, enterprise, regulated). +package policy + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/spf13/viper" +) + +// Severity controls how a rule violation is treated. +type Severity string + +const ( + SeverityError Severity = "error" + SeverityWarning Severity = "warning" + SeverityInfo Severity = "info" +) + +// Profile presets for common organizational needs. +const ( + ProfileOSS = "oss" + ProfileEnterprise = "enterprise" + ProfileRegulated = "regulated" +) + +// AIDisclosure configures AI disclosure requirements. +type AIDisclosure struct { + // Required controls whether AI disclosure is mandatory or optional. + Required bool `mapstructure:"required" json:"required"` + + // StrictToolName requires the AI tool name to be specified when AI code is present. + StrictToolName bool `mapstructure:"strict_tool_name" json:"strict_tool_name"` + + // RequireHumanReview requires human review documentation when AI code is present. + RequireHumanReview bool `mapstructure:"require_human_review" json:"require_human_review"` + + // AIBranchNaming controls how AI-prefixed branches are treated. + // Possible values: "warning", "error", "allow", "deny". + AIBranchNaming string `mapstructure:"ai_branch_naming" json:"ai_branch_naming"` +} + +// BranchConfig controls branch naming validation rules. +type BranchConfig struct { + // AllowedTypes lists the permitted branch type prefixes. + AllowedTypes []string `mapstructure:"allowed_types" json:"allowed_types"` + + // RequireTicket, when true, requires a ticket ID in the branch name. + RequireTicket bool `mapstructure:"require_ticket" json:"require_ticket"` + + // MaxDescriptionLength sets the maximum length for the description portion. + MaxDescriptionLength int `mapstructure:"max_description_length" json:"max_description_length"` +} + +// PRConfig controls pull request description validation rules. +type PRConfig struct { + // RequiredSections lists the sections that must be present in a PR body. + RequiredSections []string `mapstructure:"required_sections" json:"required_sections"` + + // MinChanges is the minimum number of change entries required. + MinChanges int `mapstructure:"min_changes" json:"min_changes"` +} + +// CommitConfig controls commit message validation rules. +type CommitConfig struct { + // AllowedTypes lists permitted conventional commit types. + AllowedTypes []string `mapstructure:"allowed_types" json:"allowed_types"` + + // RequireScope, when true, requires a scope in parentheses. + RequireScope bool `mapstructure:"require_scope" json:"require_scope"` + + // MaxSubjectLength sets the maximum length for the commit subject line. + MaxSubjectLength int `mapstructure:"max_subject_length" json:"max_subject_length"` +} + +// Policy defines the full enterprise ODS policy configuration. +type Policy struct { + // Profile selects a preset: oss, enterprise, or regulated. + Profile string `mapstructure:"profile" json:"profile"` + + // SeverityMap maps rule IDs to severity overrides. + // Keys: "branch_type", "branch_format", "pr_sections", "pr_ai_disclosure", + // "pr_ai_tool", "commit_type", "commit_scope", "commit_ai". + SeverityMap map[string]Severity `mapstructure:"severity" json:"severity"` + + // Branch controls branch naming rules. + Branch BranchConfig `mapstructure:"branch" json:"branch"` + + // PR controls pull request description rules. + PR PRConfig `mapstructure:"pr" json:"pr"` + + // Commit controls commit message rules. + Commit CommitConfig `mapstructure:"commit" json:"commit"` + + // AIDisclosure controls AI disclosure requirements. + AIDisclosure AIDisclosure `mapstructure:"ai_disclosure" json:"ai_disclosure"` +} + +// profilePresets returns hardcoded defaults for each profile. +func profilePresets() map[string]Policy { + return map[string]Policy{ + ProfileOSS: { + Profile: ProfileOSS, + Branch: BranchConfig{ + AllowedTypes: []string{"feature", "feat", "bugfix", "fix", "hotfix", "release", "chore"}, + RequireTicket: false, + MaxDescriptionLength: 100, + }, + PR: PRConfig{ + RequiredSections: []string{"## Summary", "## Type", "## Changes", "## Testing", "## Checklist"}, + MinChanges: 1, + }, + Commit: CommitConfig{ + AllowedTypes: []string{"feat", "fix", "docs", "style", "refactor", "perf", "test", "build", "ci", "chore", "revert"}, + RequireScope: false, + MaxSubjectLength: 100, + }, + AIDisclosure: AIDisclosure{ + Required: false, + StrictToolName: false, + RequireHumanReview: false, + AIBranchNaming: "warning", + }, + SeverityMap: map[string]Severity{ + "branch_type": SeverityError, + "branch_format": SeverityError, + "pr_sections": SeverityError, + "pr_ai_disclosure": SeverityWarning, + "pr_ai_tool": SeverityWarning, + "commit_type": SeverityError, + "commit_ai": SeverityWarning, + }, + }, + ProfileEnterprise: { + Profile: ProfileEnterprise, + Branch: BranchConfig{ + AllowedTypes: []string{"feature", "bugfix", "hotfix", "release", "chore"}, + RequireTicket: false, + MaxDescriptionLength: 100, + }, + PR: PRConfig{ + RequiredSections: []string{ + "## Summary", "## Type", "## AI Disclosure", + "## Changes", "## Testing", "## Checklist", + }, + MinChanges: 1, + }, + Commit: CommitConfig{ + AllowedTypes: []string{"feat", "fix", "docs", "style", "refactor", "perf", "test", "build", "ci", "chore", "revert"}, + RequireScope: true, + MaxSubjectLength: 72, + }, + AIDisclosure: AIDisclosure{ + Required: true, + StrictToolName: true, + RequireHumanReview: true, + AIBranchNaming: "warning", + }, + SeverityMap: map[string]Severity{ + "branch_type": SeverityError, + "branch_format": SeverityError, + "pr_sections": SeverityError, + "pr_ai_disclosure": SeverityError, + "pr_ai_tool": SeverityError, + "commit_type": SeverityError, + "commit_scope": SeverityWarning, + "commit_ai": SeverityError, + }, + }, + ProfileRegulated: { + Profile: ProfileRegulated, + Branch: BranchConfig{ + AllowedTypes: []string{"feature", "bugfix", "hotfix", "release", "chore"}, + RequireTicket: true, + MaxDescriptionLength: 80, + }, + PR: PRConfig{ + RequiredSections: []string{ + "## Summary", "## Type", "## AI Disclosure", + "## Related Issues", "## Changes", "## Testing", + "## Risk Assessment", "## Checklist", + }, + MinChanges: 1, + }, + Commit: CommitConfig{ + AllowedTypes: []string{"feat", "fix", "docs", "style", "refactor", "perf", "test", "build", "ci", "chore", "revert"}, + RequireScope: true, + MaxSubjectLength: 72, + }, + AIDisclosure: AIDisclosure{ + Required: true, + StrictToolName: true, + RequireHumanReview: true, + AIBranchNaming: "error", + }, + SeverityMap: map[string]Severity{ + "branch_type": SeverityError, + "branch_format": SeverityError, + "branch_ticket": SeverityError, + "pr_sections": SeverityError, + "pr_ai_disclosure": SeverityError, + "pr_ai_tool": SeverityError, + "commit_type": SeverityError, + "commit_scope": SeverityError, + "commit_ai": SeverityError, + }, + }, + } +} + +// LoadPolicy discovers and loads an ODS policy from the repository root or user config. +// It merges: profile preset < user .ods.yaml < explicit overrides. +func LoadPolicy() (*Policy, error) { + // Start with profile preset (default: enterprise) + profileName := viper.GetString("profile") + if profileName == "" || profileName == "l1" { + profileName = ProfileEnterprise + } + + presets := profilePresets() + preset, ok := presets[profileName] + if !ok { + return nil, fmt.Errorf("unknown profile: %s (valid: oss, enterprise, regulated)", profileName) + } + + // Merge user config from .ods.yaml on top of preset + merged := preset + if err := viper.UnmarshalKey("policy", &merged); err != nil { + // Config file might not exist; that's OK + } + + // Profile from viper takes precedence over config file + if p := viper.GetString("profile"); p != "" && p != "l1" { + merged.Profile = p + } + + return &merged, nil +} + +// LoadPolicyFromFile reads a policy from an explicit file path. +func LoadPolicyFromFile(path string) (*Policy, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading policy file %s: %w", path, err) + } + + // Handle the case where the file is a standalone policy YAML + // (not under a top-level "policy:" key) + v := viper.New() + v.SetConfigType("yaml") + v.SetConfigFile(path) + if err := v.ReadInConfig(); err != nil { + return nil, fmt.Errorf("parsing policy file %s: %w", path, err) + } + + // Try reading under "policy" key first, then root + var p Policy + if err := v.UnmarshalKey("policy", &p); err != nil || p.Profile == "" { + if err := v.Unmarshal(&p); err != nil { + return nil, fmt.Errorf("unmarshaling policy: %w", err) + } + } + + // Apply profile preset if specified + profileName := p.Profile + if profileName == "" { + profileName = ProfileEnterprise + } + + presets := profilePresets() + preset, ok := presets[profileName] + if !ok { + return nil, fmt.Errorf("unknown profile: %s", profileName) + } + + // Merge: preset as base, file values override + merged := preset + mergePolicies(&merged, &p) + + _ = data // used above via viper + return &merged, nil +} + +// DiscoverPolicyFile looks for an .ods.yaml file in the repo root. +func DiscoverPolicyFile() string { + searchPaths := []string{ + ".ods.yaml", + ".github/.ods.yaml", + filepath.Join(os.Getenv("HOME"), ".config", "ods", "config.yaml"), + } + for _, path := range searchPaths { + if _, err := os.Stat(path); err == nil { + return path + } + } + return "" +} + +// mergePolicies copies non-zero fields from src into dst. +func mergePolicies(dst, src *Policy) { + if src.Profile != "" { + dst.Profile = src.Profile + } + if len(src.Branch.AllowedTypes) > 0 { + dst.Branch.AllowedTypes = src.Branch.AllowedTypes + } + if src.Branch.RequireTicket { + dst.Branch.RequireTicket = true + } + if src.Branch.MaxDescriptionLength > 0 { + dst.Branch.MaxDescriptionLength = src.Branch.MaxDescriptionLength + } + if len(src.PR.RequiredSections) > 0 { + dst.PR.RequiredSections = src.PR.RequiredSections + } + if src.PR.MinChanges > 0 { + dst.PR.MinChanges = src.PR.MinChanges + } + if len(src.Commit.AllowedTypes) > 0 { + dst.Commit.AllowedTypes = src.Commit.AllowedTypes + } + if src.Commit.RequireScope { + dst.Commit.RequireScope = true + } + if src.Commit.MaxSubjectLength > 0 { + dst.Commit.MaxSubjectLength = src.Commit.MaxSubjectLength + } + if src.AIDisclosure.Required { + dst.AIDisclosure.Required = true + } + if src.AIDisclosure.StrictToolName { + dst.AIDisclosure.StrictToolName = true + } + if src.AIDisclosure.RequireHumanReview { + dst.AIDisclosure.RequireHumanReview = true + } + if src.AIDisclosure.AIBranchNaming != "" { + dst.AIDisclosure.AIBranchNaming = src.AIDisclosure.AIBranchNaming + } + if len(src.SeverityMap) > 0 { + if dst.SeverityMap == nil { + dst.SeverityMap = make(map[string]Severity) + } + for k, v := range src.SeverityMap { + dst.SeverityMap[k] = v + } + } +} + +// GetSeverity returns the severity level for a given rule ID. +func (p *Policy) GetSeverity(ruleID string) Severity { + if p.SeverityMap != nil { + if s, ok := p.SeverityMap[ruleID]; ok { + return s + } + } + // Default: all rules are errors except AI-related ones + switch { + case strings.Contains(ruleID, "ai"): + return SeverityWarning + default: + return SeverityError + } +} diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go new file mode 100644 index 0000000..1367a21 --- /dev/null +++ b/internal/policy/policy_test.go @@ -0,0 +1,191 @@ +package policy + +import ( + "os" + "path/filepath" + "testing" +) + +func TestProfilePresets(t *testing.T) { + presets := profilePresets() + + for _, name := range []string{ProfileOSS, ProfileEnterprise, ProfileRegulated} { + p, ok := presets[name] + if !ok { + t.Fatalf("missing profile: %s", name) + } + if p.Profile != name { + t.Fatalf("profile %s name mismatch: %s", name, p.Profile) + } + if len(p.Branch.AllowedTypes) == 0 { + t.Fatalf("profile %s has no branch types", name) + } + if len(p.PR.RequiredSections) == 0 { + t.Fatalf("profile %s has no PR sections", name) + } + if len(p.Commit.AllowedTypes) == 0 { + t.Fatalf("profile %s has no commit types", name) + } + } + + // OSS: AI disclosure is optional + if presets[ProfileOSS].AIDisclosure.Required { + t.Fatal("OSS profile should not require AI disclosure") + } + if presets[ProfileOSS].PR.RequiredSections[2] == "## AI Disclosure" { + t.Fatal("OSS profile should not require AI Disclosure section") + } + + // Enterprise: AI disclosure is required + if !presets[ProfileEnterprise].AIDisclosure.Required { + t.Fatal("Enterprise profile should require AI disclosure") + } + if !presets[ProfileEnterprise].Commit.RequireScope { + t.Fatal("Enterprise profile should require commit scope") + } + + // Regulated: most strict + if !presets[ProfileRegulated].Branch.RequireTicket { + t.Fatal("Regulated profile should require tickets") + } + hasRiskAssessment := false + for _, s := range presets[ProfileRegulated].PR.RequiredSections { + if s == "## Risk Assessment" { + hasRiskAssessment = true + } + } + if !hasRiskAssessment { + t.Fatal("Regulated profile should require Risk Assessment") + } +} + +func TestLoadPolicyFromFile(t *testing.T) { + policyYAML := ` +profile: regulated +branch: + allowed_types: + - feature + - bugfix + - hotfix +pr: + min_changes: 3 +ai_disclosure: + required: true + strict_tool_name: true +severity: + commit_scope: error +` + dir := t.TempDir() + path := filepath.Join(dir, ".ods.yaml") + if err := os.WriteFile(path, []byte(policyYAML), 0644); err != nil { + t.Fatal(err) + } + + p, err := LoadPolicyFromFile(path) + if err != nil { + t.Fatalf("LoadPolicyFromFile: %v", err) + } + + if p.Profile != ProfileRegulated { + t.Fatalf("profile = %s, want %s", p.Profile, ProfileRegulated) + } + if len(p.Branch.AllowedTypes) != 3 { + t.Fatalf("branch allowed_types = %d, want 3", len(p.Branch.AllowedTypes)) + } + if p.PR.MinChanges != 3 { + t.Fatalf("pr min_changes = %d, want 3", p.PR.MinChanges) + } + if !p.AIDisclosure.Required { + t.Fatal("ai_disclosure.required should be true") + } + if s := p.GetSeverity("commit_scope"); s != SeverityError { + t.Fatalf("commit_scope severity = %s, want error", s) + } + + // Inherited from regulated preset: ticket required, max description + if !p.Branch.RequireTicket { + t.Fatal("regulated profile should require tickets") + } +} + +func TestLoadPolicyFromFileOSS(t *testing.T) { + policyYAML := `profile: oss` + dir := t.TempDir() + path := filepath.Join(dir, ".ods.yaml") + if err := os.WriteFile(path, []byte(policyYAML), 0644); err != nil { + t.Fatal(err) + } + + p, err := LoadPolicyFromFile(path) + if err != nil { + t.Fatalf("LoadPolicyFromFile: %v", err) + } + + if p.Profile != ProfileOSS { + t.Fatalf("profile = %s, want %s", p.Profile, ProfileOSS) + } + if p.AIDisclosure.Required { + t.Fatal("OSS AI disclosure should not be required") + } + if p.Commit.RequireScope { + t.Fatal("OSS commit scope should not be required") + } +} + +func TestGetSeverity(t *testing.T) { + p := &Policy{ + SeverityMap: map[string]Severity{ + "branch_type": SeverityError, + "pr_ai": SeverityWarning, + }, + } + + if s := p.GetSeverity("branch_type"); s != SeverityError { + t.Fatalf("branch_type = %s, want error", s) + } + if s := p.GetSeverity("pr_ai"); s != SeverityWarning { + t.Fatalf("pr_ai = %s, want warning", s) + } + if s := p.GetSeverity("pr_ai_disclosure"); s != SeverityWarning { + t.Fatalf("unconfigured ai rule should default to warning: got %s", s) + } + if s := p.GetSeverity("branch_format"); s != SeverityError { + t.Fatalf("unconfigured non-ai rule should default to error: got %s", s) + } +} + +func TestMergePolicies(t *testing.T) { + dst := &Policy{ + Profile: ProfileOSS, + Branch: BranchConfig{ + AllowedTypes: []string{"feature"}, + }, + } + src := &Policy{ + Profile: ProfileEnterprise, + Branch: BranchConfig{ + AllowedTypes: []string{"feature", "bugfix", "hotfix"}, + RequireTicket: true, + }, + } + + mergePolicies(dst, src) + + if dst.Profile != ProfileEnterprise { + t.Fatalf("profile = %s, want %s", dst.Profile, ProfileEnterprise) + } + if len(dst.Branch.AllowedTypes) != 3 { + t.Fatalf("branch types = %d, want 3", len(dst.Branch.AllowedTypes)) + } + if !dst.Branch.RequireTicket { + t.Fatal("require_ticket should be true after merge") + } +} + +func TestDiscoverPolicyFile(t *testing.T) { + // No .ods.yaml in test dir + path := DiscoverPolicyFile() + if path != "" { + t.Logf("found policy file: %s (unexpected in test, but may exist in repo)", path) + } +} diff --git a/internal/report/report.go b/internal/report/report.go index d561f93..eed608a 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "github.com/open-delivery-spec/cli/internal/policy" "github.com/open-delivery-spec/cli/internal/validator" ) @@ -44,28 +45,31 @@ type Inputs struct { } type Check struct { - ID string `json:"id"` - Name string `json:"name"` - Status CheckStatus `json:"status"` - Score int `json:"score,omitempty"` - Value string `json:"value,omitempty"` - Notes []string `json:"notes,omitempty"` - Errors []string `json:"errors,omitempty"` - Warnings []string `json:"warnings,omitempty"` + ID string `json:"id"` + Name string `json:"name"` + Status CheckStatus `json:"status"` + Score int `json:"score,omitempty"` + Value string `json:"value,omitempty"` + Notes []string `json:"notes,omitempty"` + Errors []string `json:"errors,omitempty"` + Warnings []string `json:"warnings,omitempty"` + FixSuggestions []validator.FixSuggestion `json:"fix_suggestions,omitempty"` } type Report struct { - Title string `json:"title"` - Profile string `json:"profile"` - Status Status `json:"status"` - Score int `json:"score"` - GeneratedAt time.Time `json:"generated_at"` - Repository string `json:"repository,omitempty"` - Ref string `json:"ref,omitempty"` - SHA string `json:"sha,omitempty"` - PRNumber int `json:"pr_number,omitempty"` - BranchName string `json:"branch_name,omitempty"` - Checks []Check `json:"checks"` + Title string `json:"title"` + Profile string `json:"profile"` + PolicyProfile string `json:"policy_profile,omitempty"` + Status Status `json:"status"` + Score int `json:"score"` + GeneratedAt time.Time `json:"generated_at"` + Repository string `json:"repository,omitempty"` + Ref string `json:"ref,omitempty"` + SHA string `json:"sha,omitempty"` + PRNumber int `json:"pr_number,omitempty"` + BranchName string `json:"branch_name,omitempty"` + Checks []Check `json:"checks"` + Policy *policy.Policy `json:"policy,omitempty"` } type Options struct { @@ -110,34 +114,43 @@ func Build(in Inputs, opts Options) Report { generatedAt = time.Now().UTC() } + // Load policy for report metadata + p, err := policy.LoadPolicy() + policyProfile := "enterprise" + if err == nil && p != nil { + policyProfile = p.Profile + } + r := Report{ - Title: "ODS Compliance Report", - Profile: "L1", - GeneratedAt: generatedAt, - Repository: in.Repository, - Ref: in.Ref, - SHA: in.SHA, - PRNumber: in.PRNumber, - BranchName: in.BranchName, - Checks: buildChecks(in, opts), + Title: "ODS Compliance Report", + Profile: "L1", + PolicyProfile: policyProfile, + Policy: p, + GeneratedAt: generatedAt, + Repository: in.Repository, + Ref: in.Ref, + SHA: in.SHA, + PRNumber: in.PRNumber, + BranchName: in.BranchName, + Checks: buildChecks(in, opts, p), } r.Score, r.Status = summarize(r.Checks) return r } -func buildChecks(in Inputs, opts Options) []Check { +func buildChecks(in Inputs, opts Options, p *policy.Policy) []Check { switch opts.Check { case "branch-naming": - return []Check{validateBranch(in.BranchName, opts.Strict)} + return []Check{validateBranch(in.BranchName, opts.Strict, p)} case "commit-message": - return []Check{validateCommit(in.CommitMessage, opts.Strict)} + return []Check{validateCommit(in.CommitMessage, opts.Strict, p)} case "pr-description": - return []Check{validatePR(in.PRBody, opts.Strict)} + return []Check{validatePR(in.PRBody, opts.Strict, p)} default: return []Check{ - validateBranch(in.BranchName, opts.Strict), - validateCommit(in.CommitMessage, opts.Strict), - validatePR(in.PRBody, opts.Strict), + validateBranch(in.BranchName, opts.Strict, p), + validateCommit(in.CommitMessage, opts.Strict, p), + validatePR(in.PRBody, opts.Strict, p), } } } @@ -190,7 +203,11 @@ func Markdown(r Report) (string, error) { fmt.Fprintf(&b, "## ODS Compliance Report\n\n") fmt.Fprintf(&b, "Status: %s %s \n", statusIcon(r.Status), statusLabel(r.Status)) fmt.Fprintf(&b, "Score: %d / 100 \n", r.Score) - fmt.Fprintf(&b, "Profile: %s - AI-aware delivery metadata\n", r.Profile) + policyDisplay := r.PolicyProfile + if policyDisplay == "" { + policyDisplay = "enterprise" + } + fmt.Fprintf(&b, "Policy: `%s` - %s\n", policyDisplay, policyDescription(policyDisplay)) if r.Repository != "" || r.Ref != "" || r.SHA != "" || r.PRNumber > 0 { fmt.Fprintf(&b, "\n") } @@ -211,17 +228,52 @@ func Markdown(r Report) (string, error) { for _, check := range r.Checks { fmt.Fprintf(&b, "| %s | %s %s | %s |\n", check.Name, checkIcon(check.Status), checkLabel(check.Status), escapeMarkdownNotes(check)) } + + // Add fix suggestions section if any checks failed or have warnings + hasFixes := false + for _, check := range r.Checks { + if len(check.FixSuggestions) > 0 && (check.Status == CheckFail || check.Status == CheckWarning) { + hasFixes = true + break + } + } + if hasFixes { + fmt.Fprintf(&b, "\n---\n\n## šŸ”§ Fix Suggestions\n\n") + for _, check := range r.Checks { + if len(check.FixSuggestions) == 0 || (check.Status != CheckFail && check.Status != CheckWarning) { + continue + } + fmt.Fprintf(&b, "### %s\n\n", check.Name) + for i, fs := range check.FixSuggestions { + fmt.Fprintf(&b, "**%d. %s**\n", i+1, fs.Title) + if fs.Description != "" { + fmt.Fprintf(&b, "%s\n\n", fs.Description) + } + if fs.Template != "" { + fmt.Fprintf(&b, "```\n%s\n```\n\n", fs.Template) + } + } + } + } + return b.String(), nil } func HTML(r Report) (string, error) { tmpl := template.Must(template.New("report").Funcs(template.FuncMap{ - "statusLabel": statusLabel, - "statusIcon": statusIcon, - "checkLabel": checkLabel, - "checkIcon": checkIcon, - "joinNotes": joinNotes, - "shortSHA": shortSHA, + "statusLabel": statusLabel, + "statusIcon": statusIcon, + "checkLabel": checkLabel, + "checkIcon": checkIcon, + "joinNotes": joinNotes, + "joinNotesHTML": joinNotesHTML, + "shortSHA": shortSHA, + "hasFixSuggestions": hasFixSuggestions, + "fixSuggestionCount": fixSuggestionCount, + "formatFixTemplate": formatFixTemplate, + "scoreColor": scoreColor, + "scoreWidth": scoreWidth, + "add": func(a, b int) int { return a + b }, }).Parse(reportHTML)) var b bytes.Buffer if err := tmpl.Execute(&b, r); err != nil { @@ -231,8 +283,23 @@ func HTML(r Report) (string, error) { } func SVG(r Report) string { - label := "ODS" - message := fmt.Sprintf("%s %d", strings.ReplaceAll(statusLabel(r.Status), " ", "-"), r.Score) + policyLabel := r.PolicyProfile + if policyLabel == "" { + policyLabel = "enterprise" + } + // Shorten policy label for badge + switch policyLabel { + case "oss": + policyLabel = "OSS" + case "enterprise": + policyLabel = "ENT" + case "regulated": + policyLabel = "REG" + } + + label := fmt.Sprintf("ODS-%s", policyLabel) + statusText := statusLabel(r.Status) + message := fmt.Sprintf("%s %d/100", statusText, r.Score) color := "brightgreen" switch r.Status { case StatusCompliantWithWarnings: @@ -240,54 +307,79 @@ func SVG(r Report) string { case StatusNonCompliant: color = "red" } - return fmt.Sprintf(` + + // Adjust width based on message length + labelWidth := 70 + msgWidth := len(message)*7 + 20 + totalWidth := labelWidth + msgWidth + msgX := labelWidth + + return fmt.Sprintf(` %s: %s - + - - - + + + - %s - %s - %s - %s + %s + %s + %s + %s -`, html.EscapeString(label), html.EscapeString(message), html.EscapeString(label), html.EscapeString(message), badgeColor(color), html.EscapeString(label), html.EscapeString(label), html.EscapeString(message), html.EscapeString(message)) +`, + totalWidth, html.EscapeString(label), html.EscapeString(message), + html.EscapeString(label), html.EscapeString(message), + totalWidth, + labelWidth, + labelWidth, msgWidth, badgeColor(color), + totalWidth, + labelWidth/2, html.EscapeString(label), + labelWidth/2, html.EscapeString(label), + msgX+msgWidth/2, html.EscapeString(message), + msgX+msgWidth/2, html.EscapeString(message), + ) } -func validateBranch(value string, strict bool) Check { +func validateBranch(value string, strict bool, p *policy.Policy) Check { if strings.TrimSpace(value) == "" { return skipped("branch-naming", "Branch naming", "branch name not detected") } - result, err := validator.ValidateBranch(strings.TrimSpace(value)) + result, err := validator.ValidateBranchWithPolicy(strings.TrimSpace(value), p) return checkFromResult("branch-naming", "Branch naming", value, result, err, strict) } -func validateCommit(value string, strict bool) Check { +func validateCommit(value string, strict bool, p *policy.Policy) Check { if strings.TrimSpace(value) == "" { return skipped("commit-message", "Commit message", "commit message not detected") } - result, err := validator.ValidateCommitMessage(value) + result, err := validator.ValidateCommitMessageWithPolicy(value, p) return checkFromResult("commit-message", "Commit message", firstLine(value), result, err, strict) } -func validatePR(value string, strict bool) Check { +func validatePR(value string, strict bool, p *policy.Policy) Check { if strings.TrimSpace(value) == "" { return skipped("pr-description", "PR description", "PR body not detected") } - result, err := validator.ValidatePRDescription(value) + result, err := validator.ValidatePRDescriptionWithPolicy(value, p) return checkFromResult("pr-description", "PR description", "", result, err, strict) } func checkFromResult(id, name, value string, result validator.Result, err error, strict bool) Check { - check := Check{ID: id, Name: name, Value: value, Errors: result.Errors, Warnings: result.Warnings} + check := Check{ + ID: id, + Name: name, + Value: value, + Errors: result.Errors, + Warnings: result.Warnings, + FixSuggestions: result.FixSuggestions, + } if err != nil && result.Status != validator.StatusNonConformant { check.Errors = append(check.Errors, err.Error()) } @@ -470,11 +562,24 @@ func shortSHA(value string) string { func statusLabel(status Status) string { switch status { case StatusCompliant: - return "ODS L1 Compliant" + return "ODS Compliant" case StatusCompliantWithWarnings: - return "ODS L1 Compliant with Warnings" + return "ODS Compliant with Warnings" default: - return "ODS L1 Non-Compliant" + return "ODS Non-Compliant" + } +} + +func policyDescription(profile string) string { + switch profile { + case policy.ProfileOSS: + return "Open-source friendly; AI disclosure optional" + case policy.ProfileEnterprise: + return "Full ODS L1 enforcement with AI disclosure required" + case policy.ProfileRegulated: + return "Maximum compliance; tickets required, all AI rules enforced" + default: + return "Custom policy configuration" } } @@ -529,6 +634,55 @@ func joinNotes(check Check) string { return strings.Join(notes, "; ") } +func joinNotesHTML(check Check) template.HTML { + parts := make([]string, 0) + for _, e := range check.Errors { + parts = append(parts, fmt.Sprintf(`%s`, html.EscapeString(e))) + } + for _, w := range check.Warnings { + parts = append(parts, fmt.Sprintf(`%s`, html.EscapeString(w))) + } + if len(parts) == 0 { + if check.Value != "" { + parts = append(parts, html.EscapeString(check.Value)) + } else { + parts = append(parts, "ā€”") + } + } + return template.HTML(strings.Join(parts, "
")) +} + +func hasFixSuggestions(check Check) bool { + return len(check.FixSuggestions) > 0 && (check.Status == CheckFail || check.Status == CheckWarning) +} + +func scoreColor(score int) string { + switch { + case score >= 80: + return "#067647" + case score >= 50: + return "#b54708" + default: + return "#b42318" + } +} + +func scoreWidth(score int) string { + return fmt.Sprintf("%d%%", score) +} + +func formatFixTemplate(tmpl string) template.HTML { + return template.HTML(fmt.Sprintf("
%s
", html.EscapeString(tmpl))) +} + +func fixSuggestionCount(checks []Check) int { + count := 0 + for _, c := range checks { + count += len(c.FixSuggestions) + } + return count +} + func escapeMarkdownNotes(check Check) string { note := strings.ReplaceAll(joinNotes(check), "\n", " ") note = strings.ReplaceAll(note, "|", "\\|") @@ -705,25 +859,192 @@ const reportHTML = ` {{ .Title }} @@ -731,26 +1052,86 @@ const reportHTML = `

{{ .Title }}

- Generated {{ .GeneratedAt.Format "2006-01-02 15:04:05 MST" }}{{ if .Repository }} for {{ .Repository }}{{ end }}{{ if .Ref }} on {{ .Ref }}{{ end }}{{ if .SHA }} at {{ shortSHA .SHA }}{{ end }}{{ if .PRNumber }} for PR #{{ .PRNumber }}{{ end }} + Generated {{ .GeneratedAt.Format "2006-01-02 15:04:05 MST" }} + {{- if .Repository }} · {{ .Repository }}{{ end }} + {{- if .Ref }} on {{ .Ref }}{{ end }} + {{- if .SHA }} at {{ shortSHA .SHA }}{{ end }} + {{- if .PRNumber }} · PR #{{ .PRNumber }}{{ end }}
+
-
Status{{ statusIcon .Status }} {{ statusLabel .Status }}
-
Score{{ .Score }} / 100
-
Profile{{ .Profile }}
+
+ Status + {{ statusIcon .Status }} {{ statusLabel .Status }} +
+
+ Score + {{ .Score }} / 100 +
+
+ Policy + {{ .PolicyProfile }} +
+
+ Fix Suggestions + {{ fixSuggestionCount .Checks }} +
- - - - {{ range .Checks }} - - - - - + +
+

Compliance Score

+
+
+
+
+ 0255075100 +
+
+ +
+

Check Results

+
CheckResultNotes
{{ .Name }}{{ checkIcon .Status }} {{ checkLabel .Status }}{{ joinNotes . }}
+ + + {{ range .Checks }} + + + + + + {{ end }} + +
CheckResultDetails
{{ .Name }}{{ checkIcon .Status }} {{ checkLabel .Status }}{{ joinNotesHTML . }}
+ + + {{ if gt (fixSuggestionCount .Checks) 0 }} +
+

šŸ”§ Fix Suggestions

+

+ Copy and paste the templates below to fix the issues. +

+ {{ range .Checks }} + {{ if hasFixSuggestions . }} +

{{ .Name }}

+ {{ range $i, $fs := .FixSuggestions }} +
+

{{ add 1 $i }}. {{ $fs.Title }}

+
{{ $fs.Description }}
+ {{ if $fs.Template }}
{{ $fs.Template }}
{{ end }} + +
+ {{ end }} {{ end }} - - + {{ end }} +
+ {{ end }} + +
+ Policy: {{ .PolicyProfile }} · + Report: {{ .Title }} · + Generated: {{ .GeneratedAt.Format "2006-01-02 15:04:05 MST" }} +
diff --git a/internal/report/report_test.go b/internal/report/report_test.go index a704c6c..8d9a3af 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -164,11 +164,13 @@ func TestHTMLRendererEscapesCheckNotes(t *testing.T) { if err != nil { t.Fatalf("HTML() error = %v", err) } - if strings.Contains(page, "feature/