Skip to content

Commit 2b3c6ca

Browse files
committed
fixes from review
1 parent 0cc2756 commit 2b3c6ca

2 files changed

Lines changed: 7 additions & 3 deletions

File tree

.claude/rules/go-style.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ Applies to all Go files in the project.
1818
Check these whenever adding a method to an interface or defining a new type:
1919

2020
- **Minimal surface**: Don't add interface methods that duplicate the semantics of existing ones. If an existing method already answers the question (possibly with a side effect), don't add a separate method for the same check.
21-
- **No no-op implementations**: If a method is only meaningful for some implementations of an interface, the interface is too broad. A no-op silently breaks callers that depend on the method working. Narrow the interface or use a separate capability interface.
21+
- **No silent no-ops**: A no-op that silently breaks callers who depend on the method working is a sign the interface is too broad. Narrow the interface or use a separate capability interface. Benign no-ops (e.g., `Close()` on an in-memory store) are fine.
22+
- **Option pattern must be compile-time safe**: Options should not have runtime failures. Never define a local anonymous interface inside an option and type-assert against it — a silent no-op results if the target doesn't implement it. Two typesafe approaches:
23+
- *Config struct field*: put the setting on the config struct (e.g., `types.Config.SessionStorage`) so all consumers see it at compile time.
24+
- *Typed functional option*: use `func(*ConcreteType)` so the option only compiles against the correct receiver.
25+
If you need to cast inside an option to check whether the target supports it, the option is on the wrong abstraction. See #4638.
2226
- **Avoid parallel types that drift**: Don't define a separate config/data type that mirrors an existing one. Embed or reuse the original — two parallel structs require a conversion step and will diverge over time.
2327

2428
## Resource Leaks

.claude/rules/vmcp-anti-patterns.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,6 @@ Adding caches, connection pools, or other performance optimizations without evid
8484

8585
Adding a mutex to a domain object and mutating it in place when state changes. This grows in complexity with every new mutation and makes objects harder to reason about under concurrency.
8686

87-
**Detect**: Mutex fields on domain structs; mutation methods on types that were previously read-only; in-place writes guarded by an object-level lock.
87+
**Detect**: Mutex fields on domain structs; mutation methods on types that were previously read-only; in-place writes guarded by an object-level lock; multiple layers each holding their own mutex.
8888

89-
**Instead**: Prefer immutable objects. When state changes, replace the object rather than mutating it — rebuild from the source of truth and let readers always work with a consistent snapshot.
89+
**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; everything below that layer is then single-threaded and much easier to reason about. Sharded locks for performance should only be introduced after profiling shows contention (see anti-pattern #9).

0 commit comments

Comments
 (0)