From 5c25aea81b2d321c112ca045a340ee6a3aa1378f Mon Sep 17 00:00:00 2001 From: Mladen Todorovic Date: Tue, 28 Apr 2026 16:47:26 +0200 Subject: [PATCH] Code Rabbit config --- .claude/skills/use-modern-go/SKILL.md | 291 ++++++++++++++++++++++++++ .coderabbit.yaml | 157 ++++++++++++++ 2 files changed, 448 insertions(+) create mode 100644 .claude/skills/use-modern-go/SKILL.md create mode 100644 .coderabbit.yaml diff --git a/.claude/skills/use-modern-go/SKILL.md b/.claude/skills/use-modern-go/SKILL.md new file mode 100644 index 0000000..3755a1a --- /dev/null +++ b/.claude/skills/use-modern-go/SKILL.md @@ -0,0 +1,291 @@ +--- +name: use-modern-go +description: Apply modern Go syntax guidelines based on project's Go version. Use when user ask for modern Go code guidelines. +--- + +# Modern Go Guidelines + +## Detected Go Version + +!`grep -rh "^go " --include="go.mod" . 2>/dev/null | cut -d' ' -f2 | sort | uniq -c | sort -nr | head -1 | xargs | cut -d' ' -f2 | grep . || echo unknown` + +## How to Use This Skill + +DO NOT search for go.mod files or try to detect the version yourself. Use ONLY the version shown above. + +**If version detected (not "unknown"):** +- Say: "This project is using Go X.XX, so I’ll stick to modern Go best practices and freely use language features up to and including this version. If you’d prefer a different target version, just let me know." +- Do NOT list features, do NOT ask for confirmation + +**If version is "unknown":** +- Say: "Could not detect Go version in this repository" +- Use AskUserQuestion: "Which Go version should I target?" → [1.23] / [1.24] / [1.25] / [1.26] + +**When writing Go code**, use ALL features from this document up to the target version: +- Prefer modern built-ins and packages (`slices`, `maps`, `cmp`) over legacy patterns +- Never use features from newer Go versions than the target +- Never use outdated patterns when a modern alternative is available + +--- + +## Features by Go Version + +### Go 1.0+ + +- `time.Since`: `time.Since(start)` instead of `time.Now().Sub(start)` + +### Go 1.8+ + +- `time.Until`: `time.Until(deadline)` instead of `deadline.Sub(time.Now())` + +### Go 1.13+ + +- `errors.Is`: `errors.Is(err, target)` instead of `err == target` (works with wrapped errors) + +### Go 1.18+ + +- `any`: Use `any` instead of `interface{}` +- `bytes.Cut`: `before, after, found := bytes.Cut(b, sep)` instead of Index+slice +- `strings.Cut`: `before, after, found := strings.Cut(s, sep)` + +### Go 1.19+ + +- `fmt.Appendf`: `buf = fmt.Appendf(buf, "x=%d", x)` instead of `[]byte(fmt.Sprintf(...))` +- `atomic.Bool`/`atomic.Int64`/`atomic.Pointer[T]`: Type-safe atomics instead of `atomic.StoreInt32` + +```go +var flag atomic.Bool +flag.Store(true) +if flag.Load() { ... } + +var ptr atomic.Pointer[Config] +ptr.Store(cfg) +``` + +### Go 1.20+ + +- `strings.Clone`: `strings.Clone(s)` to copy string without sharing memory +- `bytes.Clone`: `bytes.Clone(b)` to copy byte slice +- `strings.CutPrefix/CutSuffix`: `if rest, ok := strings.CutPrefix(s, "pre:"); ok { ... }` +- `errors.Join`: `errors.Join(err1, err2)` to combine multiple errors +- `context.WithCancelCause`: `ctx, cancel := context.WithCancelCause(parent)` then `cancel(err)` +- `context.Cause`: `context.Cause(ctx)` to get the error that caused cancellation + +### Go 1.21+ + +**Built-ins:** +- `min`/`max`: `max(a, b)` instead of if/else comparisons +- `clear`: `clear(m)` to delete all map entries, `clear(s)` to zero slice elements + +**slices package:** +- `slices.Contains`: `slices.Contains(items, x)` instead of manual loops +- `slices.Index`: `slices.Index(items, x)` returns index (-1 if not found) +- `slices.IndexFunc`: `slices.IndexFunc(items, func(item T) bool { return item.ID == id })` +- `slices.SortFunc`: `slices.SortFunc(items, func(a, b T) int { return cmp.Compare(a.X, b.X) })` +- `slices.Sort`: `slices.Sort(items)` for ordered types +- `slices.Max`/`slices.Min`: `slices.Max(items)` instead of manual loop +- `slices.Reverse`: `slices.Reverse(items)` instead of manual swap loop +- `slices.Compact`: `slices.Compact(items)` removes consecutive duplicates in-place +- `slices.Clip`: `slices.Clip(s)` removes unused capacity +- `slices.Clone`: `slices.Clone(s)` creates a copy + +**maps package:** +- `maps.Clone`: `maps.Clone(m)` instead of manual map iteration +- `maps.Copy`: `maps.Copy(dst, src)` copies entries from src to dst +- `maps.DeleteFunc`: `maps.DeleteFunc(m, func(k K, v V) bool { return condition })` + +**sync package:** +- `sync.OnceFunc`: `f := sync.OnceFunc(func() { ... })` instead of `sync.Once` + wrapper +- `sync.OnceValue`: `getter := sync.OnceValue(func() T { return computeValue() })` + +**context package:** +- `context.AfterFunc`: `stop := context.AfterFunc(ctx, cleanup)` runs cleanup on cancellation +- `context.WithTimeoutCause`: `ctx, cancel := context.WithTimeoutCause(parent, d, err)` +- `context.WithDeadlineCause`: Similar with deadline instead of duration + +### Go 1.22+ + +**Loops:** +- `for i := range n`: `for i := range len(items)` instead of `for i := 0; i < len(items); i++` +- Loop variables are now safe to capture in goroutines (each iteration has its own copy) + +**cmp package:** +- `cmp.Or`: `cmp.Or(flag, env, config, "default")` returns first non-zero value + +```go +// Instead of: +name := os.Getenv("NAME") +if name == "" { + name = "default" +} +// Use: +name := cmp.Or(os.Getenv("NAME"), "default") +``` + +**reflect package:** +- `reflect.TypeFor`: `reflect.TypeFor[T]()` instead of `reflect.TypeOf((*T)(nil)).Elem()` + +**net/http:** +- Enhanced `http.ServeMux` patterns: `mux.HandleFunc("GET /api/{id}", handler)` with method and path params +- `r.PathValue("id")` to get path parameters + +### Go 1.23+ + +- `maps.Keys(m)` / `maps.Values(m)` return iterators +- `slices.Collect(iter)` not manual loop to build slice from iterator +- `slices.Sorted(iter)` to collect and sort in one step + +```go +keys := slices.Collect(maps.Keys(m)) // not: for k := range m { keys = append(keys, k) } +sortedKeys := slices.Sorted(maps.Keys(m)) // collect + sort +for k := range maps.Keys(m) { process(k) } // iterate directly +``` + +**time package** + +- `time.Tick`: Use `time.Tick` freely — as of Go 1.23, the garbage collector can recover unreferenced tickers, even if they haven't been stopped. The Stop method is no longer necessary to help the garbage collector. There is no longer any reason to prefer NewTicker when Tick will do. + +### Go 1.24+ + +- `t.Context()` not `context.WithCancel(context.Background())` in tests. + ALWAYS use t.Context() when a test function needs a context. + +Before: +```go +func TestFoo(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + result := doSomething(ctx) +} +``` +After: +```go +func TestFoo(t *testing.T) { + ctx := t.Context() + result := doSomething(ctx) +} +``` + +- `omitzero` not `omitempty` in JSON struct tags. + ALWAYS use omitzero for time.Duration, time.Time, structs, slices, maps. + +Before: +```go +type Config struct { + Timeout time.Duration `json:"timeout,omitempty"` // doesn't work for Duration! +} +``` +After: +```go +type Config struct { + Timeout time.Duration `json:"timeout,omitzero"` +} +``` + +- `b.Loop()` not `for i := 0; i < b.N; i++` in benchmarks. + ALWAYS use b.Loop() for the main loop in benchmark functions. + +Before: +```go +func BenchmarkFoo(b *testing.B) { + for i := 0; i < b.N; i++ { + doWork() + } +} +``` +After: +```go +func BenchmarkFoo(b *testing.B) { + for b.Loop() { + doWork() + } +} +``` + +- `strings.SplitSeq` not `strings.Split` when iterating. + ALWAYS use SplitSeq/FieldsSeq when iterating over split results in a for-range loop. + +Before: +```go +for _, part := range strings.Split(s, ",") { + process(part) +} +``` +After: +```go +for part := range strings.SplitSeq(s, ",") { + process(part) +} +``` +Also: `strings.FieldsSeq`, `bytes.SplitSeq`, `bytes.FieldsSeq`. + +### Go 1.25+ + +- `wg.Go(fn)` not `wg.Add(1)` + `go func() { defer wg.Done(); ... }()`. + ALWAYS use wg.Go() when spawning goroutines with sync.WaitGroup. + +Before: +```go +var wg sync.WaitGroup +for _, item := range items { + wg.Add(1) + go func() { + defer wg.Done() + process(item) + }() +} +wg.Wait() +``` +After: +```go +var wg sync.WaitGroup +for _, item := range items { + wg.Go(func() { + process(item) + }) +} +wg.Wait() +``` + +### Go 1.26+ + +- `new(val)` not `x := val; &x` — returns pointer to any value. + Go 1.26 extends new() to accept expressions, not just types. + Type is inferred: new(0) → *int, new("s") → *string, new(T{}) → *T. + DO NOT use `x := val; &x` pattern — always use new(val) directly. + DO NOT use redundant casts like new(int(0)) — just write new(0). + Common use case: struct fields with pointer types. + +Before: +```go +timeout := 30 +debug := true +cfg := Config{ + Timeout: &timeout, + Debug: &debug, +} +``` +After: +```go +cfg := Config{ + Timeout: new(30), // *int + Debug: new(true), // *bool +} +``` + +- `errors.AsType[T](err)` not `errors.As(err, &target)`. + ALWAYS use errors.AsType when checking if error matches a specific type. + +Before: +```go +var pathErr *os.PathError +if errors.As(err, &pathErr) { + handle(pathErr) +} +``` +After: +```go +if pathErr, ok := errors.AsType[*os.PathError](err); ok { + handle(pathErr) +} +``` diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000..17849db --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,157 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +# +# CodeRabbit configuration for stackrox-mcp +# +# This MCP server bridges AI assistants to StackRox/RHACS Central via gRPC. +# The project is Go-only (~120 files), with Dockerfiles, Helm charts, shell +# scripts, GitHub Actions, and Tekton/Konflux pipelines. + +inheritance: true + +reviews: + # Assertive catches more issues; acceptable for a small, security-focused project. + profile: assertive + high_level_summary: true + # Sequence diagrams help reviewers follow MCP tool→gRPC call→response flows. + sequence_diagrams: true + auto_review: + # Enabled so every PR gets automatic review — the primary goal of this config. + enabled: true + # Review drafts too — catches issues before PRs are marked ready. + drafts: true + base_branches: + - "main" + - "release-.*" + + # ── Path-based review guidance ────────────────────────────────────── + # + # Tailored for a Go MCP server that talks to StackRox Central via gRPC. + # No frontend, protobuf, Python, or database migrations in this project. + + path_instructions: + # Go is the only application language in this project. + - path: "**/*.go" + instructions: > + Go MCP server codebase. Review for: + - Proper error wrapping with client.NewError for user-facing errors + - Context propagation (context.Context as first param) + - MCP tool handlers must follow (ctx, req, input) → (*CallToolResult, *output, error) signature + - Tools must implement the toolsets.Tool interface (IsReadOnly, GetTool, GetName, RegisterWith) + - Auth context injection via auth.WithMCPRequestContext(ctx, req) + - gRPC connections via client.ReadyConn(ctx), not direct dials + - No direct fmt.Print or os.Stdout (use structured logging with `slog`) + - Testify assert/require in tests, table-driven subtests preferred + - For table-driven subtest cases use `map` with `testName` as key + - When looping over test cases, use variables `testName` and `testCase`. + - Deferred mutex unlocks instead of manual Unlock calls + - Cursor-based pagination must use the cursor package + + # Dockerfiles: standard and Konflux-specific. + - path: "**/{Dockerfile,Dockerfile.*,*.Dockerfile,konflux.Dockerfile}" + instructions: > + Container images for the MCP server. Review for: + - Minimal base images (UBI9 preferred for RHACS ecosystem) + - Multi-stage builds to minimize final image size + - No secrets or credentials in build args or layers + - Correct layer ordering for cache efficiency + - CGO_ENABLED=0 for static Go binaries + - Non-root user in the final stage + + # Shell scripts for build automation and CI. + - path: "**/*.sh" + instructions: > + Shell scripts for build and CI. Review for: + - set -euo pipefail at the top + - Proper variable quoting ("${VAR}" not $VAR) + - No hardcoded credentials, tokens, or internal URLs + - Correct exit code propagation + - Consistent use of the sandbox env vars (GOPATH, GOCACHE, etc.) + + # GitHub Actions workflows. + - path: ".github/workflows/**" + instructions: > + GitHub Actions CI/CD workflows. Review for: + - Pin action versions to full SHA, not tags (supply chain safety) + - Minimize GITHUB_TOKEN permissions (principle of least privilege) + - No script injection from untrusted PR inputs (title, body, labels) + - Secrets must use GitHub secrets, never hardcoded values + - Cache keys should include go.sum hash for correctness + + # Tekton/Konflux pipelines for RHACS CI. + - path: ".tekton/**" + instructions: > + Tekton/Konflux pipeline definitions for Red Hat CI. Review for: + - Task parameter validation and correct workspace bindings + - No hardcoded image references (use params or bundles) + - Resource limits set on task containers + - Pipeline results propagated correctly for Konflux integration + + # Helm charts for deployment. + - path: "**/helm/**" + instructions: > + Helm chart templates and values. Review for: + - Template correctness (proper quoting, indentation with nindent) + - Sensible defaults in values.yaml + - Security context set (non-root, read-only root filesystem) + - Resource requests and limits defined + - No secrets in default values + + # YAML configs (catch-all for non-workflow, non-Tekton YAML). + - path: "**/*.{yml,yaml}" + instructions: > + YAML configuration files. Review for well-formed structure, + no trailing whitespace, and correct indentation. For CI configs, + verify environment variables and secrets are handled securely. + + # E2E and integration test configs. + - path: "e2e-tests/**" + instructions: > + E2E test infrastructure using mcpchecker and WireMock. Review for: + - Correct tool patterns matching actual tool names + - WireMock stubs matching expected gRPC responses + - Test isolation (no shared mutable state between test cases) + + # ── Static analysis tools ───────────────────────────────────────────── + # + # These complement the project's own Makefile lint targets (golangci-lint, + # hadolint, shellcheck, helm-lint, actionlint). Running them in PR review + # catches issues before CI even starts. + + tools: + # Primary Go linter — matches the project's `make lint` target. + golangci-lint: + enabled: true + # Shell script analysis — matches `make shell-lint`. + shellcheck: + enabled: true + # YAML validation for configs and CI definitions. + yamllint: + enabled: true + # Dockerfile best practices — matches `make dockerfile-lint`. + hadolint: + enabled: true + # SAST scanner for security patterns (SQL injection, command injection, etc.). + semgrep: + enabled: true + # Secret detection — critical for a security product's own codebase. + gitleaks: + enabled: true + +chat: + # Auto-reply to reviewer comments for faster feedback loops. + auto_reply: true + +knowledge_base: + # Learn from the project's own conventions over time. + code_guidelines: + enabled: true + filePatterns: + - ".claude/skills/use-modern-go/SKILL.md" + learnings: + scope: auto + issues: + scope: auto + pull_requests: + scope: auto + web_search: + enabled: true