Add coding conventions derived from recent PR review feedback

[yrobla]

Summary

Captures patterns flagged in reviews across interface design, testing, operator development, and PR hygiene — so they apply automatically going forward rather than being caught in review.

Fixes #

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Changes

File	Change

Does this introduce a user-facing change?

Special notes for reviewers

[Copilot]

Pull request overview

This PR updates the repository’s Claude rule docs to codify recurring review feedback (interface design, testing practices, operator status consistency, and PR hygiene) so those expectations are enforced consistently going forward.

Changes:

• Add new Go style guidance for interface design, URL validation, bounded caches, and concurrency comment accuracy.
• Expand testing rules with E2E expectations, discouraging test hooks in production structs, and requiring timeouts for blocking concurrency barriers.
• Add operator and PR-creation guidance around status condition parity and keeping PRs single-scope.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
.claude/rules/vmcp-anti-patterns.md	Adds a new vMCP anti-pattern covering mutex-protected mutable domain objects.
.claude/rules/testing.md	Tightens test naming guidance and adds new sections for E2E behavior coverage and concurrency/testing patterns.
.claude/rules/pr-creation.md	Adds a “PR Scope” section to discourage mixed-scope PRs.
.claude/rules/operator.md	Adds guidance to keep status conditions consistent across parallel CRD types.
.claude/rules/go-style.md	Adds new Go conventions around interfaces, string constants, URL validation, bounded caches, and concurrency comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.85%. Comparing base (f5d8015) to head (0cc2756).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4636   +/-   ##
=======================================
  Coverage   68.85%   68.85%           
=======================================
  Files         505      505           
  Lines       52425    52425           
=======================================
+ Hits        36096    36099    +3     
+ Misses      13536    13534    -2     
+ Partials     2793     2792    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jerm-dro]

suggestion: The "No no-op implementations" rule is good but may be too absolute. A no-op Close() on an in-memory store is perfectly fine — the problem is when a no-op silently breaks callers' expectations. Consider softening to: "A no-op that silently breaks callers who depend on the method working is a sign the interface is too broad."

[jerm-dro]

suggestion: This section should include guidance on the option pattern, which is a common source of silent failures. We encountered a case in #4638 where a transport option defines a local anonymous interface, type-asserts against an unexported method, and silently no-ops if the target doesn't implement it.

Options should not have runtime failures — they should be validated at compile time. Two typesafe approaches:

• Config struct field: put the setting on the config struct (like types.Config.SessionStorage), so all consumers see it at compile time
• Typed functional option: use func(*ConcreteType) so the option only compiles against the correct receiver

There should be no interface casting in option application. If you need to cast to check whether the target supports the option, the option is on the wrong abstraction.

[jerm-dro]

suggestion: The anti-pattern description is on the right track but undersells the core principle. The key question isn't "mutex or no mutex" — it's can the object be reconstructed instead of mutated? If state changes, prefer rebuilding from the source of truth and replacing the reference. This eliminates an entire class of concurrency bugs.

Beyond that, the guidance should address synchronization layering: multiple layers each holding their own mutex is a smell. Prefer centralizing synchronization at a single layer — this creates a clear contract that everything below is single-threaded, which is much easier to reason about. The only exception is sharding mutexes for performance, and that should be introduced after measurement shows contention, not preemptively (aligns with anti-pattern #9).

Suggested rewrite for the "Instead" section:

Instead: Ask whether the object can be reconstructed rather than mutated — rebuild from the source of truth and replace the reference. If mutation is truly necessary, centralize synchronization at one layer rather than distributing mutexes across multiple layers. Sharded locks for performance should only be introduced after profiling shows contention.