mirror of
https://github.com/redis/go-redis.git
synced 2026-10-22 19:49:00 +03:00
chore(ai): add claude skills (#3855)
* chore(ai): add claude skills * add readme for claude configs * chore(ai): add /check-ci slash command * chore(ai): share repo guidance via AGENTS.md Move the layout/commands/architecture/conventions guidance into AGENTS.md so non-Claude agents (Codex, Cursor, Aider, ...) read the same source. CLAUDE.md now imports it via @AGENTS.md and keeps only Claude-specific notes (skills auto-trigger, slash commands). Commands are inlined rather than delegated to skills, since other tools cannot read skills. * chore(ai): add prepare-release skill Playbook to stage a release locally (semver pick, PR gathering, RELEASE-NOTES.md per template, scripts/release.sh bump, scripts/tag.sh dry-run) without ever tagging, pushing, or committing. List it in AGENTS.md; drop CLAUDE.md's duplicate skill enumeration so names live only in AGENTS.md. * update add command skill. * chore(ai): grant allowed-tools to skills add-command: WebFetch (fetch spec + docs). prepare-release: Bash(git describe:*), Bash(gh pr list:*). * chore(ai): update settings.json * docs(ai): fix test commands and stale references - ginkgo focus example used ./..., which fails every non-ginkgo package with 'flag provided but not defined' - TestPoolNew does not exist; point at TestConnStateMachine - use real CI check names in /check-ci and lead with test-redis-ce - correct keyless-command example, backoff wording, file pointers; swap drifting line refs for symbol anchors - normalize settings.json permission style; drop personal caveman block from .gitignore and the /loop reference --------- Co-authored-by: ofekshenawa <104765379+ofekshenawa@users.noreply.github.com>
This commit is contained in:
Nedyalko Dyakov
GitHub
25
.claude/README.md
Normal file
25
.claude/README.md
Normal file
@@ -0,0 +1,25 @@
|
||||
# `.claude/`
|
||||
|
||||
Claude Code configuration for the go-redis repository.
|
||||
|
||||
## `settings.json` is project-shared
|
||||
|
||||
`settings.json` is committed and applies to **every maintainer's** Claude Code
|
||||
session. Keep it to team-wide policy only — permission allowlists and other
|
||||
settings that should hold for everyone working in this repo.
|
||||
|
||||
Per-user preferences (personal model choice, local permission grants, machine
|
||||
-specific paths, etc.) belong in `settings.local.json`, which is gitignored.
|
||||
Do not add them to `settings.json`. Additions here are a deliberate policy
|
||||
choice for the whole team.
|
||||
|
||||
> Note: `settings.json` is schema-validated by Claude Code and does not accept
|
||||
> comments or unknown keys, which is why this guidance lives in the README
|
||||
> rather than inline.
|
||||
|
||||
## Layout
|
||||
|
||||
- `settings.json` — shared, committed Claude Code settings.
|
||||
- `commands/` — repo-specific slash commands (e.g. `check-ci`).
|
||||
- `skills/` — repo-specific skills (e.g. `add-command`).
|
||||
- `specs/` — architecture specs referenced by skills and by maintainers.
|
||||
27
.claude/commands/check-ci.md
Normal file
27
.claude/commands/check-ci.md
Normal file
@@ -0,0 +1,27 @@
|
||||
---
|
||||
description: Summarize GitHub Actions CI status for the current branch's PR (build, lint, e2e, govulncheck, ...)
|
||||
argument-hint: "[pr-number]"
|
||||
allowed-tools: Bash(gh pr checks:*), Bash(gh pr view:*), Bash(gh run view:*), Bash(gh pr list:*), Bash(git branch:*)
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
- Branch: !`git branch --show-current`
|
||||
- PR: !`gh pr view $1 --json number,title,url,state 2>/dev/null || echo "NO_PR"`
|
||||
- Checks (name / state / elapsed / url): !`gh pr checks $1 2>&1 || true`
|
||||
|
||||
## Task
|
||||
|
||||
Report the CI state for this PR. If the PR line is `NO_PR`, say there is no open PR for this branch and stop.
|
||||
|
||||
Otherwise, from the checks block:
|
||||
|
||||
1. **Counts first.** One line: `N passing · M failing · K pending` (and skipped/cancelled if any).
|
||||
2. **Failures and pending only.** For each check that is not passing, list its name, state, and the log URL. Skip the passing ones, they are noise.
|
||||
3. **Prioritize.** Order failures by what blocks a merge most: `test-redis-ce` (the main correctness matrix) and `E2E Tests (Mock Proxy)` first, then `lint` and `govulncheck`, then `CodeQL` / `doctests` / `check-spelling`, then everything else (benchmarks last).
|
||||
4. **Verdict.** One closing line:
|
||||
- All green → "All checks pass, nothing to do."
|
||||
- Any red → name the single most important failing check to look at first.
|
||||
- Only pending → say which are still running and that it is not done yet.
|
||||
|
||||
Keep it short. No preamble, no restating this prompt. The argument `$1` is an optional PR number; when omitted, the current branch's PR is used.
|
||||
24
.claude/settings.json
Normal file
24
.claude/settings.json
Normal file
@@ -0,0 +1,24 @@
|
||||
{
|
||||
"permissions": {
|
||||
"allow": [
|
||||
"Bash(go version)",
|
||||
"Bash(go env:*)",
|
||||
"Bash(go list:*)",
|
||||
"Bash(go doc:*)",
|
||||
"Bash(docker info)",
|
||||
"Read",
|
||||
"Grep",
|
||||
"Glob",
|
||||
"Edit",
|
||||
"Write",
|
||||
"Bash(make:*)",
|
||||
"Bash(go test:*)",
|
||||
"Bash(go build:*)",
|
||||
"Bash(go vet:*)",
|
||||
"Bash(gofumpt:*)",
|
||||
"Bash(git status)",
|
||||
"Bash(git diff:*)",
|
||||
"Bash(git log:*)"
|
||||
]
|
||||
}
|
||||
}
|
||||
78
.claude/skills/add-command/SKILL.md
Normal file
78
.claude/skills/add-command/SKILL.md
Normal file
@@ -0,0 +1,78 @@
|
||||
---
|
||||
name: add-command
|
||||
description: Use when adding a new Redis command (or RediSearch / TimeSeries / VectorSet / module subcommand) to go-redis — covers fetching the command spec and docs, the Cmder type, Cmdable interface wiring, RESP parsing, tests, and the custom-vet rule that enforces SetVal.
|
||||
allowed-tools: WebFetch
|
||||
---
|
||||
|
||||
# Adding a new Redis command
|
||||
|
||||
Router for adding a Redis command to the root `redis` package. Read the reference file for the area you're touching — don't load all of them.
|
||||
|
||||
## Step 0 — Get the command spec AND docs FIRST
|
||||
|
||||
Before writing any Go, know the exact command shape (arguments, optional flags, reply structure, since-version, key positions) **and** its documented semantics (what each reply field means, RESP2-vs-RESP3 differences, examples). Fetch **two** sources — the machine-readable spec and the prose docs. They cover different gaps: the spec nails arguments and key positions, the docs nail what the reply actually looks like.
|
||||
|
||||
### A. Machine-readable spec (arguments, key specs, reply schema)
|
||||
|
||||
Resolve the spec in this order:
|
||||
|
||||
1. **Spec file argument** — if the user passed a path to a spec/JSON file, read it.
|
||||
2. **PR URL argument** — if the user passed a `github.com/redis/redis` PR (or other repo) URL, fetch the diff and read the command definition + `src/commands/<cmd>.json` it adds.
|
||||
3. **No argument** — fetch the machine-readable spec from redis/redis:
|
||||
```
|
||||
https://raw.githubusercontent.com/redis/redis/unstable/src/commands/<command>.json
|
||||
```
|
||||
Container subcommands use `<container>-<sub>.json` (e.g. `client-info.json`). **A 404 means it's a module command** (RediSearch, TimeSeries, VectorSet, Bloom) — those specs live in the **module's own repo**, not redis/redis. Switch to the module repo (see `references/module-commands.md` §1) or ask the user for the spec/PR. Don't retry the redis/redis URL.
|
||||
|
||||
### B. Prose docs (semantics, return value, RESP2/RESP3 reply, examples)
|
||||
|
||||
The JSON `reply_schema` is often thin or missing; the docs spell out what the reply actually is — including separate **RESP2 Reply** and **RESP3 Reply** sections, which decide your `readReply` and any module RESP2-vs-RESP3 handling. Fetch the command's doc page:
|
||||
|
||||
```
|
||||
https://redis.io/docs/latest/commands/<command>/
|
||||
```
|
||||
|
||||
Raw markdown source (alternative, good for diffing or when the rendered page is noisy):
|
||||
`https://raw.githubusercontent.com/redis/redis-doc/master/commands/<command>.md`.
|
||||
**Module commands** (RediSearch, TimeSeries, …) are documented under their own
|
||||
path on redis.io (e.g. `/docs/latest/commands/ft.search/`) or in the module repo
|
||||
— see `references/module-commands.md`.
|
||||
|
||||
Read the **Return value / RESP2 Reply / RESP3 Reply** sections plus the examples, and reconcile them against the JSON `reply_schema`. When the two disagree, the docs' reply description (and a quick `redis-cli` check) win.
|
||||
|
||||
### Map both sources to the implementation
|
||||
|
||||
| Source | Drives |
|
||||
|--------|--------|
|
||||
| spec `arguments` | method signature, args-slice build order, optional `FooArgs` struct |
|
||||
| spec `reply_schema` + docs **Return value** | the `readReply` parser and Cmder result type |
|
||||
| docs **RESP2 Reply / RESP3 Reply** | RESP2-vs-RESP3 branching in `readReply` (see `references/module-commands.md`) |
|
||||
| spec `since` + docs `@history` | `SkipBeforeRedisVersion(...)` in the integration test, doc comment |
|
||||
| spec `key_specs` | key-position maps in `command.go`; cluster routing |
|
||||
| spec `command_flags` (e.g. `READONLY`, no key) | keyless / fan-out handling, cluster routing |
|
||||
| docs examples | integration-test cases and expected values |
|
||||
|
||||
If you can't get either source, STOP and ask the user — guessing the reply shape produces a broken `readReply`.
|
||||
|
||||
## Decide before you start
|
||||
|
||||
1. **Which `*_commands.go` file?** — pick the existing file matching the data type (`string_commands.go`, `hash_commands.go`, `search_commands.go`, …). Only create a new file for a genuinely new category. New files need a matching `XxxCmdable` interface embedded in `Cmdable` (`commands.go`).
|
||||
2. **Reuse an existing Cmder, or define a new one?** — simple replies (`int`, `string`, `bool`, `[]string`, map) reuse `*IntCmd`, `*StringCmd`, `*BoolCmd`, `*StringSliceCmd`, `*MapStringStringCmd`, … Define a new Cmder only for a structured reply that no existing type fits.
|
||||
3. **`Foo` vs `FooWithArgs`?** — for commands with optional flags, expose a positional `Foo(...)` for the common path plus `FooWithArgs(ctx, key, *FooArgs)` for the full surface.
|
||||
|
||||
## Reference files
|
||||
|
||||
| Read this | When |
|
||||
|-----------|------|
|
||||
| `references/core-command-pattern.md` | Always — the 7-step Go pattern (interface method, Cmder type, RESP parsing, tests, vet, fmt). Worked example: `LCS`. |
|
||||
| `references/module-commands.md` | Adding a RediSearch / TimeSeries / VectorSet / Bloom subcommand — naming, RESP2-vs-RESP3 shape differences, where the Cmd type lives. |
|
||||
| `references/cluster-routing-wiring.md` | Command is keyless, fans out, multi-key, or aggregates across shards. |
|
||||
|
||||
When several apply, read in order: core → module → cluster.
|
||||
|
||||
## Pitfalls (full list in core-command-pattern.md)
|
||||
|
||||
- Forgetting to embed the new `XxxCmdable` interface in `Cmdable` — compiles on `*Client`, silently unreachable via `UniversalClient`.
|
||||
- Forgetting `Clone()` — pipelines reuse Cmders; shared `val` causes cross-execution bugs.
|
||||
- Skipping `SetVal` because "nothing calls it" — hooks do, and the `setval` custom-vet check fails the build.
|
||||
- Using `time.Duration` directly in args — server gets nanoseconds. Convert per spec.
|
||||
@@ -0,0 +1,29 @@
|
||||
# Cluster routing for new commands
|
||||
|
||||
Read this only when the new command is keyless, fans out, targets multiple keys, or aggregates results across shards. **Most simple single-key commands need no cluster-side change** — slot routing is inferred from the key argument automatically.
|
||||
|
||||
This file (the skill reference) is a wiring checklist. For the full routing *design* — slot computation, MOVED/ASK, aggregator internals — read the separate architecture spec at the repo path `.claude/specs/cluster-routing.md` before changing routing code.
|
||||
|
||||
## When a command needs wiring
|
||||
|
||||
| Command shape | What to wire |
|
||||
|---------------|--------------|
|
||||
| Keyless (e.g. `DBSIZE`, `FLUSHALL`, server-info commands) | Add the lowercase name to `keylessCommands` (`command.go:25`). Otherwise `ClusterClient` can't pick a node. |
|
||||
| Unusual key position (key not the first arg) | Update the key-position / key-spec maps in `command.go`. |
|
||||
| Fan-out across all shards, results aggregated | Add a request/response policy in `command_policy_resolver.go` and the matching aggregator in `internal/routing/aggregator.go` (policy declared in `internal/routing/policy.go`). |
|
||||
| Multi-key, cross-slot | Confirm the cross-slot rules in `.claude/specs/cluster-routing.md`; commands spanning slots error unless the command supports it. |
|
||||
|
||||
## Source of truth = the command spec
|
||||
|
||||
The spec's `key_specs` and `command_flags` tell you the routing class:
|
||||
- empty `key_specs` → keyless → `keylessCommands`.
|
||||
- `command_flags` with no write flag → `READONLY` → eligible for replica routing.
|
||||
- multiple key specs / `numkeys`-style args → multi-key; check cross-slot handling.
|
||||
|
||||
## Verify
|
||||
|
||||
After wiring, exercise against the cluster profile:
|
||||
|
||||
```sh
|
||||
go test -run TestGinkgoSuite . -ginkgo.focus="cluster"
|
||||
```
|
||||
106
.claude/skills/add-command/references/core-command-pattern.md
Normal file
106
.claude/skills/add-command/references/core-command-pattern.md
Normal file
@@ -0,0 +1,106 @@
|
||||
# Core command pattern
|
||||
|
||||
The 7-step Go pattern for adding a command to the root `redis` package. Worked example: **LCS** (grep `func (c cmdable) LCS` in `string_commands.go` and `type LCSCmd struct` in `command.go`) — it uses a `cmdable` receiver, a query struct, a custom Cmder, and a registered `CmdType`. Copy its shape exactly.
|
||||
|
||||
## 1. Method on the `cmdable` interface
|
||||
|
||||
In the matching `*_commands.go` file. Receiver is `cmdable` — this is what makes the command available on every client type (`Client`, `ClusterClient`, `Ring`, sentinel, pipelines). A `*Client` receiver compiles but is unreachable via `UniversalClient` / `Cmdable`.
|
||||
|
||||
```go
|
||||
func (c cmdable) LCS(ctx context.Context, q *LCSQuery) *LCSCmd {
|
||||
cmd := NewLCSCmd(ctx, q)
|
||||
_ = c(ctx, cmd)
|
||||
return cmd
|
||||
}
|
||||
```
|
||||
|
||||
For commands with many optional flags, expose `Foo(...)` for the common path plus `FooWithArgs(ctx, key, *FooArgs)` for the full surface (see `FTHybrid` / `FTHybridWithArgs` in `search_commands.go`).
|
||||
|
||||
Conventions:
|
||||
- Lowercase the command name in the args slice (`"lcs"`, not `"LCS"`); subcommand flags are uppercase (`"LEN"`, `"IDX"`, `"MINMATCHLEN"`).
|
||||
- Pre-size the args slice (`make([]interface{}, 3, 7)`).
|
||||
- Convert `time.Duration` to whatever the server expects — never pass it raw (server gets nanoseconds). Use `formatMs` / `formatSec` in `commands.go`.
|
||||
- Doc comment: state `Redis X.Y+` (from the spec `since`) and link the redis.io command page.
|
||||
|
||||
## 2. New `XxxCmdable` interface goes into `Cmdable`
|
||||
|
||||
Only if you added a **new** `*_commands.go` file with its own interface. Embed it in `Cmdable` (`commands.go`, alphabetical in the embed list — e.g. `SearchCmdable` at `commands.go:242`). Without this the method is unreachable through `UniversalClient`.
|
||||
|
||||
## 3. Custom Cmder type
|
||||
|
||||
Define only when no existing Cmd type (`*IntCmd`, `*StringCmd`, `*BoolCmd`, `*StringSliceCmd`, `*MapStringStringCmd`, …) fits the reply. Put it at the bottom of `command.go` (module commands may keep it in their own `*_commands.go`, like `FTHybridCmd`). Follow `LCSCmd`:
|
||||
|
||||
```go
|
||||
type LCSCmd struct {
|
||||
baseCmd
|
||||
readType uint8 // command-specific parse state
|
||||
val *LCSMatch
|
||||
}
|
||||
|
||||
var _ Cmder = (*LCSCmd)(nil)
|
||||
|
||||
func NewLCSCmd(ctx context.Context, q *LCSQuery) *LCSCmd {
|
||||
args := make([]interface{}, 3, 7)
|
||||
args[0], args[1], args[2] = "lcs", q.Key1, q.Key2
|
||||
// ...append optional flags...
|
||||
cmd := &LCSCmd{readType: 1}
|
||||
cmd.baseCmd = baseCmd{ctx: ctx, args: args, cmdType: CmdTypeLCS}
|
||||
return cmd
|
||||
}
|
||||
|
||||
func (cmd *LCSCmd) SetVal(val *LCSMatch) { cmd.val = val }
|
||||
func (cmd *LCSCmd) Val() *LCSMatch { return cmd.val }
|
||||
func (cmd *LCSCmd) Result() (*LCSMatch, error) { return cmd.val, cmd.err }
|
||||
func (cmd *LCSCmd) String() string { return cmdString(cmd, cmd.val) }
|
||||
func (cmd *LCSCmd) readReply(rd *proto.Reader) error { /* parse RESP */ }
|
||||
func (cmd *LCSCmd) Clone() Cmder { /* deep-copy val */ }
|
||||
```
|
||||
|
||||
Required methods — the `setval` analyzer (`internal/customvet/checks/setval`) fails the build if `Result` exists without `SetVal`:
|
||||
- `SetVal(...)` — hooks mutate results through it. (`SetErr` is inherited from `baseCmd`.)
|
||||
- `Val()`, `Result()` — public accessors.
|
||||
- `String()` — via `cmdString`.
|
||||
- `readReply(rd *proto.Reader) error` — RESP parser. Use `rd.ReadArrayLen`, `rd.ReadInt`, `rd.ReadString`, `rd.ReadFloat`, `rd.ReadFixedMapLen`, `rd.ReadFixedArrayLen`. Validate element counts when the protocol guarantees a fixed shape. Always propagate the reader's error — never return `nil` after a partial read.
|
||||
- `Clone()` — deep-copy `val` (including slices/maps) so pipeline/transaction reuse is safe.
|
||||
|
||||
**CmdType:** set `cmdType: CmdTypeXxx` in the constructor. If you add a new `CmdTypeXxx` constant (`command.go`), also add the matching case in the `ExtractCommandValue` switch further down `command.go` — this is what lets hooks read the typed value. Grep `CmdTypeLCS` to find both the const declaration and its `case` in `ExtractCommandValue`; copy that shape.
|
||||
|
||||
## 4. RESP3 vs RESP2
|
||||
|
||||
If the reply differs between protocols, the parser must handle both — this applies to **any** command with a protocol-dependent shape, core or module, not just modules. Typical case: a map in RESP3, a flat key/value array in RESP2. Branch on the reply type with `rd.PeekReplyType()` (check `proto.RespMap` for RESP3) or `rd.IsResp3()`. Tests run RESP3 by default but RESP2 must still parse for older servers. See `references/module-commands.md` for the FTHybrid worked example.
|
||||
|
||||
## 5. Tests — both layers
|
||||
|
||||
- **Unit test** in `xxx_commands_unit_test.go` (no Redis) — assert the args slice your method builds for the interesting flag combinations. Pattern: `search_commands_unit_test.go:120` captures `m.lastCmd.(*FTHybridCmd)` and checks args.
|
||||
- **Integration test** in `xxx_commands_test.go` (Ginkgo, docker stack). Gate version-specific commands: `SkipBeforeRedisVersion(8.8, "X requires Redis 8.8+")` (version from spec `since`). Cover happy path, errors, optional-arg combos.
|
||||
|
||||
```sh
|
||||
go test -run TestFTHybridWithArgsRejectsUnsupportedVectors .
|
||||
go test -run TestGinkgoSuite . -ginkgo.focus="LCS"
|
||||
```
|
||||
|
||||
## 6. Custom vet check
|
||||
|
||||
```sh
|
||||
cd internal/customvet && go build .
|
||||
go vet -vettool ./internal/customvet/customvet ./...
|
||||
```
|
||||
|
||||
Same check CI runs at the end of `make test.ci`; catches a missing `SetVal`.
|
||||
|
||||
## 7. Format
|
||||
|
||||
```sh
|
||||
make fmt
|
||||
```
|
||||
|
||||
(`gofumpt` + `goimports -local github.com/redis/go-redis`.)
|
||||
|
||||
## Pitfalls
|
||||
|
||||
- Forgetting to embed the new `XxxCmdable` interface in `Cmdable` — compiles on `*Client`, silently unreachable via `UniversalClient`.
|
||||
- Forgetting `Clone()` — pipelines reuse Cmders; shared `val` causes cross-execution bugs.
|
||||
- Skipping `SetVal` because "nothing calls it" — hooks do, and custom-vet fails the build.
|
||||
- Returning `nil` from `readReply` after a partial read — always propagate the reader's error.
|
||||
- `time.Duration` passed raw — server gets nanoseconds; convert per spec.
|
||||
- New `CmdTypeXxx` added but not registered in the type-routing switch in `command.go`.
|
||||
49
.claude/skills/add-command/references/module-commands.md
Normal file
49
.claude/skills/add-command/references/module-commands.md
Normal file
@@ -0,0 +1,49 @@
|
||||
# Module commands (RediSearch / TimeSeries / VectorSet / Bloom)
|
||||
|
||||
Module subcommands (`FT.*`, `TS.*`, `V*`, `BF.*`/`CF.*`) follow the core pattern but with three differences. Read `references/core-command-pattern.md` first. Worked example: **FTHybrid** (`search_commands.go`).
|
||||
|
||||
## 1. Where the spec lives
|
||||
|
||||
Module command specs are **not** in `redis/redis/src/commands/`. They live in the module's own repo:
|
||||
- RediSearch → `RediSearch/RediSearch`
|
||||
- TimeSeries → `RedisTimeSeries/RedisTimeSeries`
|
||||
- VectorSet / Bloom → their respective repos
|
||||
|
||||
If Step 0's redis/redis fetch 404s, the command is a module command — get the spec from the module repo or ask the user for the spec file / PR.
|
||||
|
||||
## 2. Where the Cmder type lives
|
||||
|
||||
Module Cmd types are defined in the **module's** `*_commands.go` file, not `command.go`. `FTHybridCmd` lives in `search_commands.go` (grep `type FTHybridCmd`), registered on the `SearchCmdable` interface at the top of that file, which is embedded in `Cmdable` (`commands.go`). TimeSeries → `timeseries_commands.go`, VectorSet → `vectorset_commands.go`. The required-method set is identical to core (SetVal/Val/Result/String/readReply/Clone).
|
||||
|
||||
## 3. RESP2 vs RESP3 reply shapes
|
||||
|
||||
Module commands frequently return a **map in RESP3** and a **flat array in RESP2** for the same logical reply. The parser must handle both. FTHybrid reads the raw slice, then `parseFTHybrid` walks it tolerating three shapes:
|
||||
|
||||
```go
|
||||
func (cmd *FTHybridCmd) readReply(rd *proto.Reader) (err error) {
|
||||
data, err := rd.ReadSlice() // generic slice read
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
result, cursorResult, err := parseFTHybrid(data, cmd.withCursor)
|
||||
// ...assign cmd.val / cmd.cursorVal...
|
||||
}
|
||||
|
||||
// inside parseFTHybrid, per result item:
|
||||
if itemMap, ok := item.(map[string]interface{}); ok { // RESP3 map
|
||||
results = append(results, itemMap)
|
||||
} else if rawMap, ok := item.(map[interface{}]interface{}); ok { // alt RESP3
|
||||
// stringify keys
|
||||
} else if itemData, ok := item.([]interface{}); ok { // RESP2 array of k/v pairs
|
||||
// walk pairs
|
||||
}
|
||||
```
|
||||
|
||||
Pattern: don't assume RESP3. Try the map shape, fall back to the array (key-value pairs) shape. Tests run RESP3 by default but RESP2 must still parse for older servers.
|
||||
|
||||
## 4. Common module conventions
|
||||
|
||||
- Subcommand keyword is uppercase in args (`"SEARCH"`, `"VSIM"`, `"WITHCURSOR"`); the command token is lowercase (`"ft.hybrid"`).
|
||||
- Cursor-style commands carry parse state on the Cmd struct (FTHybrid's `withCursor`, `cursorVal`) — set it in the constructor from the options.
|
||||
- Module commands are almost always `FooWithArgs(ctx, index, *FooOptions)` plus a thin positional `Foo(...)`.
|
||||
- Version-gate integration tests against the **module** version, not the core Redis version.
|
||||
32
.claude/skills/commit-style/SKILL.md
Normal file
32
.claude/skills/commit-style/SKILL.md
Normal file
@@ -0,0 +1,32 @@
|
||||
---
|
||||
name: commit-style
|
||||
description: Use when writing a commit message or opening a PR in the go-redis repo — covers the Conventional-Commit format, the repo's scope vocabulary (pool, conn, sentinel, command, …), breaking-change syntax, and the rule that NO AI-attribution trailer is added.
|
||||
---
|
||||
|
||||
# go-redis commit messages
|
||||
|
||||
Conventional Commits, short and exact — `<type>(<scope>): <imperative summary>`. Subject ≤50 chars (hard cap 72), imperative mood ("add", not "added"), no trailing period. Body only when the *why* isn't obvious from the diff (breaking change, security fix, migration, non-obvious rationale); wrap at 72, bullets `-`. State why, not what — the diff says what.
|
||||
|
||||
- **Types:** `feat`, `fix`, `refactor`, `perf`, `docs`, `test`, `chore` (also `build`, `ci`, `style`, `revert`).
|
||||
- **Scope** = the subsystem touched, lowercase: `pool`, `conn`, `pubsub`, `sentinel`, `retry`, `command`/`cmd`, `vectorset`, `otel`, `streams`, `push`, `deps`, `ci`, `tests`, `docs`. Omit scope only for genuinely cross-cutting changes.
|
||||
- **Breaking change:** `feat(scope)!: ...` plus a `BREAKING CHANGE:` body line.
|
||||
- **Issues/PRs:** reference at the end — `Closes #42`, `Refs #17`.
|
||||
|
||||
## No attribution trailer
|
||||
|
||||
Do **not** add `Co-Authored-By: Claude …`, "Generated with Claude Code", or any AI-attribution line to commits or PR bodies in this repo. This overrides both the default harness behavior (which appends such trailers) and any other commit skill.
|
||||
|
||||
## Examples
|
||||
|
||||
```
|
||||
fix(pool): release wantConn slot on dial timeout
|
||||
|
||||
The waiter stayed queued after the dial deadline fired, leaking a
|
||||
slot per timed-out connection under load.
|
||||
|
||||
Closes #3812
|
||||
```
|
||||
|
||||
```
|
||||
feat(streams): support explicit LIMIT 0 in XTRIM trimming
|
||||
```
|
||||
103
.claude/skills/prepare-release/SKILL.md
Normal file
103
.claude/skills/prepare-release/SKILL.md
Normal file
@@ -0,0 +1,103 @@
|
||||
---
|
||||
name: prepare-release
|
||||
description: Use when preparing a go-redis release — cutting a new version, bumping version.go, updating the go-redis dependency version in submodule go.mod files, or writing the RELEASE-NOTES.md entry for a new vX.Y.Z. Covers choosing the next semver, finding the last release and gathering merged PRs since then, the release-notes format and what to exclude, running scripts/release.sh to bump versions, and verifying with the scripts/tag.sh dry-run. Does NOT create tags, push, or commit — publishing stays a manual maintainer step.
|
||||
allowed-tools: Bash(git describe:*), Bash(gh pr list:*)
|
||||
---
|
||||
|
||||
# Preparing a go-redis release
|
||||
|
||||
Stage everything for a release locally, then stop before anything is published.
|
||||
|
||||
## This skill never publishes
|
||||
|
||||
- **Never** run `scripts/tag.sh ... -t` — the `-t` flag creates and pushes git tags.
|
||||
- **Never** `git push` anything (no commits, no tags).
|
||||
- **Never** commit on the maintainer's behalf unless they explicitly ask.
|
||||
|
||||
The deliverable is a reviewable diff: bumped versions plus a new `RELEASE-NOTES.md`
|
||||
entry. A human reviews it and runs the publish step.
|
||||
|
||||
## 1. Pick the next version
|
||||
|
||||
The current version is the source of truth in `version.go`:
|
||||
|
||||
```sh
|
||||
grep 'return' version.go # e.g. return "9.21.0"
|
||||
```
|
||||
|
||||
Choose the next `vX.Y.Z` by semver, based on what shipped since the last release:
|
||||
|
||||
- **patch** (`Z`) — bug fixes only; drop-in upgrade.
|
||||
- **minor** (`Y`) — new features, no breaking changes; drop-in upgrade.
|
||||
- **major** (`X`) — breaking changes.
|
||||
|
||||
Confirm the level with the user if the changeset is ambiguous.
|
||||
|
||||
## 2. Find the last release and gather changes
|
||||
|
||||
`scripts/tag.sh` also tags every public submodule (`extra/redisotel/vX.Y.Z`, …),
|
||||
so a naive `git describe` returns a submodule tag. Match the **root** tag only:
|
||||
|
||||
```sh
|
||||
LAST=$(git describe --tags --abbrev=0 --match 'v[0-9]*') # e.g. v9.21.0
|
||||
git log "$LAST"..HEAD --oneline
|
||||
gh pr list --state merged --limit 100 \
|
||||
--json number,title,author,mergedAt,url --search "merged:>=<last-release-date>"
|
||||
```
|
||||
|
||||
Categorize the PRs: highlights, new features, bug fixes, performance,
|
||||
testing/infrastructure. Exclude dependabot bumps, typo-only doc fixes, internal
|
||||
refactors with no user-facing effect, and `dependabot[bot]` from the contributor
|
||||
list.
|
||||
|
||||
## 3. Write the release notes
|
||||
|
||||
Prepend a new `# X.Y.Z (YYYY-MM-DD)` section to the **top** of `RELEASE-NOTES.md`
|
||||
(newest first; leave older entries untouched). Follow
|
||||
[`.github/RELEASE_NOTES_TEMPLATE.md`](../../../.github/RELEASE_NOTES_TEMPLATE.md)
|
||||
exactly — section order, emoji headers, the `([#PR](url)) by [@user](url)` link
|
||||
format, and the `**Full Changelog**` compare link `${LAST}...vX.Y.Z`. Open the
|
||||
lead line with the release type and whether it is a drop-in upgrade, matching the
|
||||
existing entries.
|
||||
|
||||
That template carries the full "what to exclude" and formatting rules — read it,
|
||||
don't reinvent them. `release-drafter` separately auto-drafts a GitHub release
|
||||
from PR labels (`.github/release-drafter-config.yml`); `RELEASE-NOTES.md` is the
|
||||
curated, hand-written record and is the file you edit.
|
||||
|
||||
## 4. Bump versions
|
||||
|
||||
Run the repo's bump script. It rewrites the go-redis dependency version in every
|
||||
submodule `go.mod`, runs `go mod tidy`, and bumps `version.go`. It does **not**
|
||||
commit, push, or switch branches:
|
||||
|
||||
```sh
|
||||
TAG=vX.Y.Z ./scripts/release.sh
|
||||
```
|
||||
|
||||
## 5. Verify (dry run only)
|
||||
|
||||
Run the tag script's **dry run** — it checks that `version.go` and every `go.mod`
|
||||
already match the tag, and prints the tags it *would* push. No `-t`:
|
||||
|
||||
```sh
|
||||
./scripts/tag.sh vX.Y.Z # DRY RUN — must pass cleanly; never add -t here
|
||||
make build
|
||||
git diff --stat # review version.go, submodule go.mod, RELEASE-NOTES.md
|
||||
```
|
||||
|
||||
Fix anything the dry run flags before handing off.
|
||||
|
||||
## 6. Hand off
|
||||
|
||||
Report that the release is staged and list the publish steps for the maintainer
|
||||
to run themselves (this skill does not do them):
|
||||
|
||||
1. Review `git diff`, then commit (`chore(release): vX.Y.Z` — see the
|
||||
`commit-style` skill; no AI-attribution trailer).
|
||||
2. Open and merge the release PR.
|
||||
3. After merge, tag and push: `./scripts/tag.sh vX.Y.Z -t`.
|
||||
4. `release-drafter` publishes the GitHub release; reconcile it with the
|
||||
`RELEASE-NOTES.md` entry if needed.
|
||||
|
||||
Stop after step 5. Do not commit, tag, or push.
|
||||
62
.claude/skills/testing/SKILL.md
Normal file
62
.claude/skills/testing/SKILL.md
Normal file
@@ -0,0 +1,62 @@
|
||||
---
|
||||
name: testing
|
||||
description: Use when running go-redis tests of any kind — the full suite, a single test, or a filtered/focused subset (e.g. only cluster, sentinel, or pool specs); bringing up or tearing down the Docker test stack; the e2e maintenance-notifications suite (including the proxy-free logic tests); building or formatting; or setting the REDIS_VERSION / RE_CLUSTER / REDIS_PORT knobs that drive the test image and version gating. Covers how to focus a Ginkgo spec and how to run plain go tests.
|
||||
---
|
||||
|
||||
# Running tests, builds, and the Docker stack
|
||||
|
||||
Tests run against a Redis stack started via Docker Compose. Profiles in `docker-compose.yml` control which services come up (`standalone`, `cluster`, `sentinel`, `all`, `e2e`, …).
|
||||
|
||||
## Make targets
|
||||
|
||||
```sh
|
||||
make docker.start # bring up the full test stack (profile: all)
|
||||
make docker.stop
|
||||
make test # docker.start -> test.ci -> docker.stop
|
||||
make test.ci # run tests assuming containers are already up
|
||||
make test.ci.skip-vectorsets # used when REDIS_VERSION < 8
|
||||
make bench # go test -bench=. (root module only)
|
||||
make fmt # gofumpt + goimports (-local github.com/redis/go-redis)
|
||||
make build
|
||||
make go_mod_tidy # go mod tidy across every module
|
||||
```
|
||||
|
||||
The Makefile iterates over every `go.mod` (`GO_MOD_DIRS`) for `test.ci`, `go_mod_tidy`, etc.
|
||||
|
||||
## E2E (maintenance notifications)
|
||||
|
||||
Needs an extra `cae-resp-proxy` service:
|
||||
|
||||
```sh
|
||||
make test.e2e # auto-starts the e2e profile, runs ./maintnotifications/e2e/, tears down
|
||||
make test.e2e.docker # subset that runs inside docker
|
||||
make test.e2e.logic # logic-only tests, no proxy required
|
||||
```
|
||||
|
||||
## Env knobs (passed through the Makefile)
|
||||
|
||||
- `REDIS_VERSION` — e.g. `8.8`. Drives both the test image tag and `main_test.go` version-gating helpers (`SkipBeforeRedisVersion` / `SkipAfterRedisVersion`).
|
||||
- `CLIENT_LIBS_TEST_IMAGE` — full image ref, e.g. `redislabs/client-libs-test:8.8-m03`. (To change the default tag, use the `update-ci-image` skill — it lives in several files.)
|
||||
- `RE_CLUSTER=true` — point tests at a Redis Enterprise cluster instead of the docker-compose stack. The suite then skips ring/sentinel/TLS-cluster setup.
|
||||
- `RCE_DOCKER=true` — using Redis CE in docker (default for `make test`).
|
||||
- `REDIS_PORT` — override the default standalone port (`6380`) used by `main_test.go`.
|
||||
|
||||
## Running a single test
|
||||
|
||||
The root suite is Ginkgo-based (`bsm/ginkgo` + `bsm/gomega` forks). `go test -run` matches the Go-level wrapper, so to focus a Ginkgo spec use the Ginkgo focus flag:
|
||||
|
||||
```sh
|
||||
go test -run TestGinkgoSuite . -ginkgo.focus="ZAdd"
|
||||
go test -run TestGinkgoSuite . -ginkgo.focus="cluster"
|
||||
```
|
||||
|
||||
Plain `go test`-style tests (most files outside the Ginkgo suite, e.g. `internal/...`, `maintnotifications/...`) work the usual way:
|
||||
|
||||
```sh
|
||||
go test -run TestConnStateMachine ./internal/pool/...
|
||||
go test -race -run TestCircuitBreaker ./maintnotifications/...
|
||||
```
|
||||
|
||||
## Version-gating
|
||||
|
||||
Gate Redis-version-specific tests with `SkipBeforeRedisVersion` / `SkipAfterRedisVersion` (driven by `REDIS_VERSION`) rather than skipping at the suite level.
|
||||
71
.claude/skills/update-ci-image/SKILL.md
Normal file
71
.claude/skills/update-ci-image/SKILL.md
Normal file
@@ -0,0 +1,71 @@
|
||||
---
|
||||
name: update-ci-image
|
||||
description: Use when adding, updating, or removing a `redislabs/client-libs-test` image tag used by the test stack — covers every place a Redis version → image-tag mapping lives so local `make test` and GitHub CI stay in sync.
|
||||
---
|
||||
|
||||
# Updating the CI test image
|
||||
|
||||
The `redislabs/client-libs-test` image is referenced in **six** places (five live config + one doc). Miss one and either local `make test` and CI diverge, or a single CI job uses the wrong image. Always edit all relevant locations in one change.
|
||||
|
||||
## The six locations
|
||||
|
||||
| # | File | What lives here |
|
||||
|---|------|-----------------|
|
||||
| 1 | `Makefile` | `CLIENT_LIBS_TEST_IMAGE ?= redislabs/client-libs-test:<tag>` — default for local `make test` / `make docker.start`. |
|
||||
| 2 | `docker-compose.yml` | `x-default-image: &default-image ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:<tag>}` — fallback when the env var is unset. Should match the Makefile default. |
|
||||
| 3 | `.github/workflows/build.yml` | `redis_version_mapping` bash assoc array (`["8.X.x"]="<tag>"`). Drives the `benchmark` job and the `test-redis-ce` matrix via `run-tests`. |
|
||||
| 4 | `.github/actions/run-tests/action.yml` | Identical `redis_version_mapping` array — the composite action that the matrix jobs invoke. Keep in lockstep with `build.yml`. |
|
||||
| 5 | `.github/workflows/doctests.yaml` | Hardcoded `image: redislabs/client-libs-test:<tag>` on the `redis-stack` service — doctests do not consult the mapping. |
|
||||
| 6 | `CONTRIBUTING.md` | Prose line "By default the docker image … is `redislabs/client-libs-test:<tag>`." Documentation only — no CI impact, but goes stale silently. Update it to match the Makefile default. |
|
||||
|
||||
## Tasks
|
||||
|
||||
### Update the tag for an existing Redis version (e.g. bump 8.8 → a new release)
|
||||
|
||||
1. `Makefile` — replace the tag after `redislabs/client-libs-test:` on the `CLIENT_LIBS_TEST_IMAGE` line.
|
||||
2. `docker-compose.yml` — replace the tag in the `:-redislabs/client-libs-test:<tag>` default.
|
||||
3. `.github/workflows/build.yml` — update the `["8.X.x"]="<tag>"` line in `redis_version_mapping`.
|
||||
4. `.github/actions/run-tests/action.yml` — update the matching `["8.X.x"]="<tag>"` line.
|
||||
5. `.github/workflows/doctests.yaml` — update the `image:` line **only if** doctests should run against this version. Doctests typically pin to the latest stable; check before changing.
|
||||
|
||||
If the version that doctests run against (`Makefile` default) and the mapping for that same minor version diverge, you have a bug — they should resolve to the same tag.
|
||||
|
||||
### Add a new Redis version to the CI matrix
|
||||
|
||||
1. `.github/workflows/build.yml`:
|
||||
- Add `- "<X.Y>.x"` to **both** `redis-version` matrices (the `benchmark` job and the `test-redis-ce` job — they have separate lists).
|
||||
- Add `["<X.Y>.x"]="<tag>"` to `redis_version_mapping`.
|
||||
2. `.github/actions/run-tests/action.yml` — add the same `["<X.Y>.x"]="<tag>"` entry to its `redis_version_mapping`.
|
||||
3. If this becomes the new default for local dev, also update `Makefile` (`REDIS_VERSION ?=` and `CLIENT_LIBS_TEST_IMAGE ?=`) and `docker-compose.yml`.
|
||||
|
||||
### Remove a Redis version
|
||||
|
||||
1. `.github/workflows/build.yml` — drop the `- "<X.Y>.x"` line from both `redis-version` matrices and the `["<X.Y>.x"]="..."` entry.
|
||||
2. `.github/actions/run-tests/action.yml` — drop the same `["<X.Y>.x"]="..."` mapping entry.
|
||||
3. If `Makefile` / `docker-compose.yml` / `doctests.yaml` were pinned to this version, pick a replacement (usually the next-newest version still in the matrix) and update them too.
|
||||
|
||||
### Switch to a custom build (e.g. `custom-26172898734-debian` for an unreleased server feature)
|
||||
|
||||
Same as "update the tag," but think about scope:
|
||||
- Custom tags usually correspond to a single Redis minor — only touch the mapping entry for that minor, not all five rows.
|
||||
- Custom tags often shouldn't be the local default if they're transient. Confirm with the user before touching `Makefile` / `docker-compose.yml`.
|
||||
- Leave `REDIS_VERSION` (the numeric `8.8` / `8.6` etc.) alone — it drives `SkipBeforeRedisVersion` gating in `main_test.go`, not the image. The custom image still reports its base version.
|
||||
|
||||
## Verification
|
||||
|
||||
After editing, search for any stale tag references:
|
||||
|
||||
```sh
|
||||
# Use the Grep tool, not bash grep.
|
||||
# Pattern: literal old tag, e.g. "8.8-rc1" or "client-libs-test:8.8"
|
||||
```
|
||||
|
||||
Expected matches: zero in `Makefile`, `docker-compose.yml`, `.github/**`. A reference in `AGENTS.md` or the `testing` skill documentation (e.g. "e.g. `redislabs/client-libs-test:8.8-m03`") is illustrative — leave it unless the doc is misleading after the change.
|
||||
|
||||
## Gotchas
|
||||
|
||||
- **Two mappings, not one.** `build.yml` and `run-tests/action.yml` each declare their own `redis_version_mapping`. They are not deduped. Always edit both.
|
||||
- **Two matrices in `build.yml`.** The `benchmark` job and the `test-redis-ce` job each have their own `redis-version` list. Adding/removing a version means editing both.
|
||||
- **`doctests.yaml` is hardcoded.** It doesn't read the mapping. If the team intends "doctests follow the latest stable," verify the tag matches `Makefile`'s default after any change.
|
||||
- **Don't change `REDIS_VERSION` when only swapping the image.** The number gates version-specific test skips (`SkipBeforeRedisVersion(8.8, ...)`). A custom image built off 8.8 should keep `REDIS_VERSION=8.8`.
|
||||
- **Env var vs. default.** Local users can override with `CLIENT_LIBS_TEST_IMAGE=...`; `docker-compose.yml` uses that env var first and falls back to its baked-in default. So the docker-compose default only matters when the Makefile isn't driving things — but it's still load-bearing for direct `docker compose up` calls. Keep it in sync.
|
||||
153
.claude/specs/cluster-routing.md
Normal file
153
.claude/specs/cluster-routing.md
Normal file
@@ -0,0 +1,153 @@
|
||||
# Cluster routing
|
||||
|
||||
This is an architectural reference for how `ClusterClient` decides which node to send a command to and how it merges results from multi-shard fan-outs. Read this before changing `osscluster.go`, `osscluster_router.go`, `command_policy_resolver.go`, or anything under `internal/routing/` and `internal/hashtag/`.
|
||||
|
||||
The mental model: a command becomes a `(slot, policy, aggregator)` triple, executes against one or more nodes, and its responses are collapsed by the aggregator. Most bugs come from getting the slot wrong, picking the wrong policy, or aggregating responses incorrectly.
|
||||
|
||||
---
|
||||
|
||||
## Slot computation
|
||||
|
||||
Redis Cluster has 16384 slots. A key's slot is `CRC16(hashtag(key)) % 16384` where `hashtag(key)` returns the substring inside the first balanced `{...}` if present, otherwise the whole key (`internal/hashtag/hashtag.go`). Empty keys map to a random slot to avoid deterministic concentration.
|
||||
|
||||
Two non-obvious edge cases:
|
||||
|
||||
- **Byte-slice keys** (#3049): the cluster client used to cast args via `fmt.Sprintf("%s", v)` which doesn't handle `[]byte` consistently. `stringArg` in `command.go` now type-switches to handle `[]byte` keys passed through `client.Do(ctx, "GET", []byte("k"))`. If you add a generic-args path, route it through `stringArg`.
|
||||
- **Commands whose key is *not* the first arg, or which take the slot directly** — `COUNTKEYSINSLOT` is the canonical example: its argument is the slot integer, not a key (#3327). When you add such a command, register the correct key-spec in `command.go` or its slot will be computed from the wrong arg and the routing will land on the wrong shard.
|
||||
|
||||
`keylessCommands` in `command.go` is the override list for commands with no key at all (`PING`, `TIME`, `CONFIG`, `WAIT`, …). Routing them by "first arg" would either error or pick a random shard; instead, the router uses the configured `ShardPicker` (round-robin / random / static). Adding `WAIT` (#3615) and `TIME` (#3722) here fixed real bugs where pipelines and txpipelines mis-routed.
|
||||
|
||||
---
|
||||
|
||||
## MOVED and ASK redirection
|
||||
|
||||
`MOVED <slot> <addr>` means "this slot lives somewhere else now, permanently" — the client follows the redirect and triggers a topology reload (`loadState`). `ASK <slot> <addr>` means "this slot is *currently* migrating; just for this command, send `ASKING` then retry against `<addr>`" — no topology reload, only a one-shot redirect. Detection is via typed errors (`error.go`, `IsMovedError`/`IsAskError`) which return the address as well as a bool.
|
||||
|
||||
Two design decisions worth knowing:
|
||||
|
||||
- **No backoff after MOVED/ASK** (#3048). Redirection is informational; the new node is ready to serve. Adding exponential backoff between the redirect and the retry just adds latency for no benefit. If you're tempted to add a delay, you're probably solving a different problem (e.g. a thundering herd during failover) and should solve it elsewhere.
|
||||
- **`MASTERDOWN` is retriable** (#3164). It means a primary stepped down and a replica is being promoted; the right behaviour is to wait briefly and retry (with the existing topology), not to fail. `dial tcp` errors are now also retriable as redirects (#3786) so we don't lose connectivity during DNS flips.
|
||||
|
||||
When `MOVED` points at the same address we just used, that's a stale DNS cache, not a real redirect. The pool/dialer treats this as a bad connection and removes it (#3219 made `clusterNodes.Addrs()` return a copy so this race no longer corrupts the address list under the GC).
|
||||
|
||||
---
|
||||
|
||||
## Command policies (#3422)
|
||||
|
||||
`internal/routing/policy.go` defines the policy taxonomy. A command has a `RequestPolicy` (where it goes) and a `ResponsePolicy` (how multiple responses are combined).
|
||||
|
||||
### Request policies
|
||||
|
||||
| Policy | Meaning |
|
||||
|---|---|
|
||||
| `ReqDefault` | Single shard. Slot from first key (or the configured ShardPicker for keyless commands). |
|
||||
| `ReqAllNodes` | Broadcast to all masters **and** replicas. (`DEBUG OBJECT`, etc.) |
|
||||
| `ReqAllShards` | Broadcast to all masters only. (`DBSIZE`, `KEYS`, `SCAN` orchestration.) |
|
||||
| `ReqMultiShard` | Group keys by slot, fan out per slot. (`MGET`, `MSET`, `DEL` with cross-shard keys.) |
|
||||
| `ReqSpecial` | Custom resolver; e.g. `FT.CURSOR` is sticky-routed by cursor ID. |
|
||||
|
||||
### Response policies
|
||||
|
||||
| Policy | Meaning |
|
||||
|---|---|
|
||||
| `RespDefaultKeyless` | Return the first non-error result. |
|
||||
| `RespDefaultHashSlot` | Reassemble in original key order (for `MGET` etc.). |
|
||||
| `RespAllSucceeded` | Fail the whole call if any shard errored. |
|
||||
| `RespOneSucceeded` | Succeed if any shard succeeded. |
|
||||
| `RespAggSum` / `RespAggMin` / `RespAggMax` | Numeric aggregates. |
|
||||
| `RespAggLogicalAnd` / `RespAggLogicalOr` | Boolean aggregates. |
|
||||
| `RespSpecial` | Custom — e.g. `ZUNION` with `COUNT` (#3802). |
|
||||
|
||||
`CommandPolicy.CanBeUsedInPipeline()` returns `false` for `ReqAllNodes`, `ReqAllShards`, and `ReqMultiShard` — these can't sit inside a regular pipeline because they fan out across multiple connections. If you mark a new command with one of these request policies, expect pipeline tests to need updates.
|
||||
|
||||
`command_policy_resolver.go` resolves policies for a given command. Resolution order: explicit policy in the resolver → server-supplied tips (RESP3 `command info`) → defaults. There's a fallback chain you can extend with `SetFallbackResolver` (used by some test scaffolding and module-specific resolvers).
|
||||
|
||||
---
|
||||
|
||||
## Aggregators (`internal/routing/aggregator.go`)
|
||||
|
||||
The aggregator is the runtime side of `ResponsePolicy`. Two families:
|
||||
|
||||
- **Keyless aggregators** — collect responses without per-key bookkeeping. `DefaultKeylessAggregator`, `AllSucceededAggregator`, `AggSumAggregator`, etc.
|
||||
- **Keyed aggregators** — preserve key order. `DefaultKeyedAggregator` is the one you want for `MGET`, `MSET`, `DEL`: each shard returns results for its subset of keys, and the aggregator stitches them back in the user's original order. Reordering bugs here surface as "MGET returned values in the wrong slots."
|
||||
|
||||
When you add a new fan-out command, the typical workflow is:
|
||||
|
||||
1. Pick the right `RequestPolicy` (almost always `ReqMultiShard` for multi-key, `ReqAllShards` for global state).
|
||||
2. Pick the right `ResponsePolicy`. If none fit, register a `RespSpecial` aggregator.
|
||||
3. Test it with **at least 3 shards** and **keys distributed across all of them**, including the empty case (no keys land on a particular shard).
|
||||
|
||||
The `add aggregator case for new command` style commit shows up regularly because step 2 is easy to forget. Pair it with `add-command` in your mental checklist; the `add-command` skill points at this doc for a reason.
|
||||
|
||||
---
|
||||
|
||||
## Shard picker
|
||||
|
||||
For keyless commands and tie-breaking among replicas, `ShardPicker` (`internal/routing/shard_picker.go`) selects a node. Three implementations: round-robin (default), random, static. The picker is configured on `ClusterOptions.ShardPicker` and does *not* affect commands that have a slot.
|
||||
|
||||
The dispatch function `cmdNodeWithShardPicker` uses slot `-1` as the sentinel for "use the picker." Don't pass `-1` accidentally for a real command — that's a routing bug waiting to happen.
|
||||
|
||||
---
|
||||
|
||||
## Replica routing
|
||||
|
||||
`ReadOnly`, `RouteRandomly`, `RouteByLatency`, `ReplicaOnly` are the four read-routing knobs. Their interaction is:
|
||||
|
||||
- `ReadOnly = false` (default): always go to master.
|
||||
- `ReadOnly = true`, no other knob: go to master, but allow replicas as fallback. Reads never go to replicas in this mode unless the master is down.
|
||||
- `RouteRandomly = true`: pick a random non-failing replica for read-only commands.
|
||||
- `RouteByLatency = true`: pick the lowest-latency healthy replica.
|
||||
- `ReplicaOnly = true`: only ever talk to replicas.
|
||||
|
||||
Two history points:
|
||||
|
||||
- **Latency throttling** (#2795): `RouteByLatency` would otherwise ping every replica before every command. Latencies are now measured at most once per `minLatencyMeasurementInterval` (10s, defined in `osscluster.go`).
|
||||
- **`slotClosestNode` fail-safe** (#3043): if all replicas are unhealthy, fall back to the lowest-latency *failing* replica rather than a random one; if all pings have timed out, fall back to a random replica. This was a real bug that produced "no nodes available" errors during partial outages.
|
||||
- **PubSub on replicas** (#3480): `ClusterClient.Subscribe` can target replica nodes, with the connection running in `READONLY` mode.
|
||||
- **`ReplicaOnly` + `NewFailoverClusterClient`** (#3482): the option used to be silently dropped on failover-cluster construction. Watch for this when extending failover.
|
||||
|
||||
---
|
||||
|
||||
## Topology reload
|
||||
|
||||
`clusterState` carries `Masters`, `Slaves`, and slot→node mappings, built from `CLUSTER SLOTS`. It's stored under `clusterStateHolder` (an `atomic.Value`) so reads are lock-free. Reloads run in the background, triggered by:
|
||||
|
||||
- `MOVED` errors (canonical case).
|
||||
- Periodic refresh.
|
||||
- `SMIGRATED` notifications from the maintenance-notifications subsystem (it can target a partial reload by passing the affected slot ranges).
|
||||
|
||||
`loadState` tries every known address in random order until one returns a usable topology. If all fail, `activeAddrs` is cleared so DNS gets re-resolved on the next attempt. Stale node generations get GC'd one minute after a reload (so old node addresses don't keep accumulating).
|
||||
|
||||
The `IsClusterMode` option (#3255) flips routing behaviour for ElastiCache cluster mode, which emulates `CLUSTER SLOTS` but with cluster-mode semantics layered on a different topology. The flag exists because we can't always detect cluster mode reliably from the server response alone.
|
||||
|
||||
---
|
||||
|
||||
## Cross-slot, transactions, and pipelines
|
||||
|
||||
`ClusterClient` enforces these:
|
||||
|
||||
- **`Watch`/`MULTI`/`EXEC` (TxPipeline)**: all keys must hash to the same slot. Cross-slot returns `ErrCrossSlot` immediately and sets the error on every queued command. This is a Redis Cluster protocol limit, not a client choice.
|
||||
- **`Pipeline`**: no restriction. Commands are grouped by slot and dispatched in parallel; each shard sees only the commands targeting it. This is why `Pipeline` can succeed where `TxPipeline` would fail.
|
||||
- **`Eval`/`EvalSha`**: routed by the **first key in `KEYS`**. Scripts that read multiple keys must declare them all in `KEYS` and all of them must hash to the same slot. If your script touches a key it didn't declare, you'll get cluster-routing errors at runtime.
|
||||
- **`FT.CURSOR` and similar paginated commands**: sticky routing by cursor ID via `ReqSpecial`, so pagination stays on the node that owns the cursor.
|
||||
|
||||
---
|
||||
|
||||
## History worth knowing
|
||||
|
||||
Recurring shape of cluster bugs:
|
||||
|
||||
- **Wrong slot computation**: byte-slice keys (#3049), commands with non-key first arg (`COUNTKEYSINSLOT` #3327), missing keyless markers (`WAIT` #3615, `TIME` #3722).
|
||||
- **Race in node accounting**: `clusterNodes.Addrs()` returning a shared slice (#3219).
|
||||
- **Replica routing fallbacks**: `slotClosestNode` not handling all-failing case (#3043).
|
||||
- **Treating recoverable failures as fatal**: `MASTERDOWN` (#3164), `dial tcp` errors during failover (#3786).
|
||||
- **Unnecessary delays on recoverable redirects**: backoff after MOVED/ASK (#3048).
|
||||
- **Failover variants forgetting options**: `ReplicaOnly` (#3482), `MaintNotificationsConfig` (#3600).
|
||||
|
||||
When you add a feature or fix a bug here, add a test that:
|
||||
|
||||
1. Spins a 6-node cluster (the test scaffolding in `main_test.go` already provides `clusterScenario` with 16600–16605).
|
||||
2. Uses keys that you've verified land on different shards (use `CLUSTER KEYSLOT` if unsure).
|
||||
3. Exercises both the happy path and the redirect path.
|
||||
|
||||
The cluster integration tests are slow but catch real protocol issues — the unit-only tests don't.
|
||||
204
.claude/specs/maintnotifications.md
Normal file
204
.claude/specs/maintnotifications.md
Normal file
@@ -0,0 +1,204 @@
|
||||
# Maintenance notifications (smart client handoff)
|
||||
|
||||
This is an architectural reference for `maintnotifications/`. Read this before changing notification handling, the handoff worker pool, the per-conn state machine, or anything that touches `MaintNotificationsConfig`.
|
||||
|
||||
The mental model: Redis Enterprise pushes RESP3 notifications when nodes are about to move or fail over. The client either (a) **relaxes timeouts** so in-flight commands don't false-time-out during the disruption, or (b) **hands off** an existing TCP connection to a new endpoint without dropping pending operations. The user-visible promise is "no spurious errors during planned maintenance."
|
||||
|
||||
---
|
||||
|
||||
## Why this exists
|
||||
|
||||
During a Redis Enterprise upgrade or scale event, individual nodes get drained, slots get migrated, and primaries fail over to replicas. Without help, a client sees this as connection drops, slow commands, and timeout errors. With maintenance notifications, the server tells the client *what's about to happen* via RESP3 push frames, and the client compensates: extends timeouts during the disruption, opens a new connection to the new endpoint, and migrates pending commands across.
|
||||
|
||||
This only works on Redis Enterprise (and Redis Cloud) — open-source Redis doesn't emit these notifications. It also only works on RESP3 (`Protocol: 3`), since RESP2 has no concept of out-of-band push frames.
|
||||
|
||||
---
|
||||
|
||||
## Notifications and what they mean
|
||||
|
||||
Two families of frames:
|
||||
|
||||
### Per-connection (standalone clients)
|
||||
|
||||
| Frame | Meaning | Client action |
|
||||
|---|---|---|
|
||||
| `["MOVING", seqID, timeS, endpoint]` | This connection's node is being drained; reconnect to `endpoint` within `timeS`. | Hand off the connection. |
|
||||
| `["MIGRATING", seqID, ...]` | A slot migration is starting; expect latency spikes. | Apply `RelaxedTimeout`. |
|
||||
| `["MIGRATED", seqID, ...]` | Migration done. | Clear relaxed timeout. |
|
||||
| `["FAILING_OVER", seqID, ...]` | This primary is about to fail over. | Apply `RelaxedTimeout`. |
|
||||
| `["FAILED_OVER", seqID, ...]` | Failover done. | Clear relaxed timeout. |
|
||||
|
||||
`MOVING` is the only frame that triggers a real connection handoff. The others only adjust timeouts on the existing connection.
|
||||
|
||||
### Cluster-level
|
||||
|
||||
| Frame | Meaning | Client action |
|
||||
|---|---|---|
|
||||
| `["SMIGRATING", seqID, slot/range, ...]` | Cluster slot migration starting. | Relax timeouts on connections to the source node. |
|
||||
| `["SMIGRATED", seqID, [[src, dst, slots], ...]]` | Cluster slot migration done. | Clear relaxed timeouts; trigger lazy `clusterState` reload (selectively, using the slot ranges). |
|
||||
|
||||
ClusterClient supports only `SMIGRATING`/`SMIGRATED`. Per-connection `MOVING` is meaningless in cluster mode because cluster clients discover endpoint changes via `MOVED`/`ASK` redirection — the cluster routing path already handles node moves. Adding `MOVING` to the cluster path would duplicate (and potentially conflict with) the routing layer's redirection logic.
|
||||
|
||||
`SMIGRATED` deduplication is by `seqID` (`Manager.MarkSMigratedSeqIDProcessed`) — Redis can re-send the same notification on multiple connections, and we only want to reload topology once.
|
||||
|
||||
---
|
||||
|
||||
## Modes
|
||||
|
||||
`MaintNotificationsConfig.Mode`:
|
||||
|
||||
- `ModeDisabled` — feature off; no handshake sent. Choose this when running against open-source Redis.
|
||||
- `ModeEnabled` — `CLIENT MAINT_NOTIFICATIONS ON` is mandatory. Hard error if the server rejects it.
|
||||
- `ModeAuto` (default) — try the handshake, silently downgrade if the server doesn't understand it.
|
||||
|
||||
The handshake lives inside `initConn`. The order is: `HELLO 3` → `CLIENT MAINT_NOTIFICATIONS ON`. If `HELLO 3` fails (e.g., RESP2-only server), `ModeAuto` skips the second step and the connection runs without notifications. `ModeEnabled` errors out — the user explicitly asked for the feature.
|
||||
|
||||
Three real bugs trace to this handshake:
|
||||
|
||||
- **#3788** — `ModeAuto` would still attempt the maintenance handshake after `HELLO` failed, panicking on the unexpected reply shape.
|
||||
- **#3707** — `MaintNotificationsConfig` could be nil (when disabled), and `initConn` dereferenced it. The fix nil-checks before every access; preserve that pattern when adding fields.
|
||||
- **#3789** — Endpoint-type auto-detection was flaky over slow DNS; a 2-second resolution timeout and clearer IP-range classification (RFC1918, RFC4193, loopback, RFC6598) fixed it.
|
||||
|
||||
---
|
||||
|
||||
## Per-connection state
|
||||
|
||||
`maintnotifications/state.go` defines a tiny manager-level enum: `StateIdle` (no upgrade in progress) and `StateMoving` (handoff initiated). The richer per-connection state lives in `internal/pool` via the conn state machine — `StateUnusable` is what the pool uses to keep handoff-targeted connections out of `Get` while the worker is operating on them.
|
||||
|
||||
The flow for a `MOVING` notification on conn X:
|
||||
|
||||
1. Push handler reads `["MOVING", seqID, timeS, endpoint]` and calls `conn.MarkForHandoff(endpoint, seqID)` on X.
|
||||
2. The next `Put` to return X to the pool sees `ShouldHandoff()` true and routes X to `MarkQueuedForHandoff` (transitioning to `StateUnusable`) and enqueues a handoff request.
|
||||
3. The pool no longer hands X out via `Get` (the `OnGet` hook rejects unusable conns).
|
||||
4. A handoff worker picks up the request, dials the new endpoint, runs `initConn` against the new socket, and swaps the underlying conn.
|
||||
5. On success, `ClearHandoffState()` returns the conn to `StateIdle`. On terminal failure, the worker removes it.
|
||||
|
||||
The whole sequence is async w.r.t. the user's request that triggered the `Put`. The user does not block on handoff.
|
||||
|
||||
---
|
||||
|
||||
## Handoff worker pool (`handoff_worker.go`)
|
||||
|
||||
Workers are created on demand and self-terminate after 15 seconds idle. Auto-sizing (when not explicitly configured):
|
||||
|
||||
```
|
||||
MaxWorkers = min(PoolSize/2, max(10, PoolSize/3))
|
||||
HandoffQueueSize = max(20 * MaxWorkers, PoolSize)
|
||||
capped by min(MaxActiveConns + 1, 5 * PoolSize)
|
||||
```
|
||||
|
||||
Examples:
|
||||
- `PoolSize=100` → 33 workers, 660 queue (capped at 500).
|
||||
- `PoolSize=100, MaxActiveConns=150` → 33 workers, 151 queue.
|
||||
|
||||
These bounds are tuned to absorb a "whole pool moves at once" event — during a node drain you can get a `MOVING` notification on every connection at roughly the same time.
|
||||
|
||||
### Queue-full behavior
|
||||
|
||||
If the handoff queue is full, the worker manager waits 100ms for space; if still full, it returns `ErrHandoffQueueFull` and the connection is removed from the pool (no retry at this layer). It will be re-dialed by the pool when needed. **This is intentional**: queueing forever would mask a fundamental misconfiguration, and dropping the conn is recoverable.
|
||||
|
||||
### Retries
|
||||
|
||||
A failed handoff retries up to `MaxHandoffRetries` (default 3). The retry delay is flat, not exponential: the remainder of the handoff deadline, falling back to `HandoffTimeout/3` when no deadline is set, with a 500ms floor. Some errors are non-retryable (e.g., `initConn` failure — the new endpoint is unreachable or rejecting auth). After max retries, the conn is permanently removed.
|
||||
|
||||
### Post-handoff relaxed duration
|
||||
|
||||
After a successful handoff, the *new* conn has its read/write deadlines extended for `PostHandoffRelaxedDuration` (default `2 * RelaxedTimeout`). Why: even after the new endpoint accepts the connection, the cluster is still settling — first commands may be slow as topology propagates. The relaxed window covers the post-stabilization tail. This is applied *before* `initConn` so the auth handshake also benefits.
|
||||
|
||||
### Past pitfall
|
||||
|
||||
#3633 — the worker's logging path could call `closeConnFromRequest` with a nil error and then `err.Error()` panic-ed. Always nil-check errors before logging in this code; the worker runs on a long-lived goroutine and a panic kills future handoffs.
|
||||
|
||||
---
|
||||
|
||||
## Pool hook integration (`pool_hook.go`)
|
||||
|
||||
The maintnotifications subsystem plugs into the pool via `pool.PoolHook`:
|
||||
|
||||
- `OnGet` — reject the conn if `IsUsable()` is false or `ShouldHandoff()` is true.
|
||||
- `OnPut` — if the conn needs handoff, queue it (transitioning to `StateUnusable` so the pool keeps it but won't hand it out).
|
||||
- `OnRemove` / `Shutdown` — clean up.
|
||||
|
||||
`enableMaintNotificationsUpgrades` (called from `redis.go` after `initConn` confirms RESP3) constructs the `Manager`, registers the pool hook, and registers push handlers on `push.Registry`. On client `Close`, the manager calls `pool.RemovePoolHook` and shuts down its workers.
|
||||
|
||||
The "ReAuth must not interfere with handoff" invariant (#3547) lives at this layer: both subsystems use `StateUnusable`, so whichever one transitions first wins and the other observes the unusable state via `OnGet`. See [pool.md](pool.md) for the full coexistence contract.
|
||||
|
||||
---
|
||||
|
||||
## Circuit breaker (`circuit_breaker.go`)
|
||||
|
||||
Per-endpoint circuit breaker keyed by destination. States: `Closed` (allow), `Open` (reject), `HalfOpen` (probe). Defaults: open after 5 consecutive failures, half-open after 60s, close after 3 consecutive successes from half-open.
|
||||
|
||||
Workflow: before a handoff dial, check `IsOpen(endpoint)`. If open, fail fast — no point burning the retry budget on an endpoint that's known-broken. After each handoff, record the result.
|
||||
|
||||
A background goroutine GCs unused circuit breakers (>30 min unused) every 5 minutes so a long-lived client doesn't accumulate state for one-off endpoint failures.
|
||||
|
||||
---
|
||||
|
||||
## Endpoint type resolution (`config.go`)
|
||||
|
||||
`EndpointType` selects which form the client uses when reconnecting after a `MOVING` notification (Redis Enterprise emits multiple endpoint forms in topology):
|
||||
|
||||
- `EndpointTypeAuto` — auto-detect based on the current connection (TLS preference, IP ranges).
|
||||
- `EndpointTypeInternalIP` / `EndpointTypeInternalFQDN`.
|
||||
- `EndpointTypeExternalIP` / `EndpointTypeExternalFQDN`.
|
||||
- `EndpointTypeNone` — reconnect with the originally configured address.
|
||||
|
||||
Auto-detection logic in `DetectEndpointType`:
|
||||
|
||||
- TLS enabled → prefer FQDN (so SNI and cert validation work).
|
||||
- TLS disabled → prefer IP (avoids unnecessary DNS).
|
||||
- Internal vs external is decided by IP range (RFC1918 / RFC4193 / loopback / RFC6598) for IP literals, or by resolution-and-classification for hostnames (with a 2-second DNS timeout, post #3789).
|
||||
|
||||
Choose explicitly if your network topology requires a specific form — e.g., a client running outside the VPC must use `EndpointTypeExternalFQDN`.
|
||||
|
||||
---
|
||||
|
||||
## Manager (`manager.go`)
|
||||
|
||||
Central coordinator. Owns:
|
||||
|
||||
- The push notification handler registrations (one per notification type).
|
||||
- A `sync.Map` of in-flight `MOVING` operations keyed by conn ID, so duplicate notifications don't double-handoff.
|
||||
- The `SMIGRATED` seqID dedup set.
|
||||
- The cluster-state-reload callback (set by `ClusterClient` so the manager can trigger a partial reload on `SMIGRATED` without holding a back-reference to the cluster client).
|
||||
|
||||
Lock-free state where possible: `atomic.Int64` for active operation count, `atomic.Bool` for closed flag. Callbacks fire on the push notification handler goroutine — keep them fast and avoid blocking I/O directly inside them.
|
||||
|
||||
---
|
||||
|
||||
## E2E testing
|
||||
|
||||
`maintnotifications/e2e/` has its own `go.mod` and a separate test entrypoint. It drives a fault-injecting RESP proxy (`cae-resp-proxy`) that sits between the client and Redis and can synthesize maintenance notifications on demand. Scenarios are written as Go tests that:
|
||||
|
||||
1. Start the proxy with a `DEFAULT_INTERCEPTORS` profile (`cluster`, `hitless`).
|
||||
2. Create a real client.
|
||||
3. Issue commands while the proxy injects `MOVING`/`SMIGRATING`/etc.
|
||||
4. Assert no spurious errors and correct handoff timing.
|
||||
|
||||
Run with `make test.e2e` (auto-starts the proxy via the `e2e` docker profile). `make test.e2e.logic` runs only the logic-layer tests (no proxy). `make test.e2e.docker` runs the proxy-based subset that fits in CI's docker-network constraints.
|
||||
|
||||
If you're changing the subsystem in ways that could affect handoff timing, run E2E locally — unit tests don't catch races between notification arrival and request lifecycle.
|
||||
|
||||
---
|
||||
|
||||
## History worth knowing
|
||||
|
||||
Recurring failure modes:
|
||||
|
||||
- **Nil `MaintNotificationsConfig`** dereferenced in `initConn` when the feature is disabled (#3707). Always nil-check before access.
|
||||
- **HELLO failure not handled** before the maintenance handshake (#3788). The handshake assumes RESP3 — if `HELLO 3` failed, skip it.
|
||||
- **Endpoint-type detection over slow DNS** (#3789). Use the bounded resolver, not bare `net.LookupHost`.
|
||||
- **Failover client wiring gaps** (#3600). When you add fields to `MaintNotificationsConfig`, check that `FailoverClient` and `FailoverClusterClient` propagate them.
|
||||
- **Pool/handoff/re-auth interaction** (#3547). Always use `StateUnusable` for exclusive ownership, never a separate flag.
|
||||
- **Logging panics on nil error** (#3633) in long-lived worker goroutines. Defensive nil-checks in log paths.
|
||||
- **Test flakes from notification-injection timing** (#3641). Synchronize injection with client state, don't rely on sleeps.
|
||||
|
||||
If you add a new notification type, the checklist is roughly:
|
||||
|
||||
1. Define the frame shape and document it in `README.md` / this file.
|
||||
2. Add a parser in `push_notification_handler.go`.
|
||||
3. Decide: per-connection action (mark-for-handoff, relax timeout) or manager-level (topology reload, dedup)?
|
||||
4. If per-connection: route through the pool hook so the existing state-machine guarantees apply. Don't invent a new exclusion mechanism.
|
||||
5. Add an E2E scenario in `maintnotifications/e2e/`.
|
||||
6. Decide whether `ClusterClient` should also handle it — if it duplicates routing-layer behaviour (`MOVED`/`ASK`), probably no.
|
||||
146
.claude/specs/pool.md
Normal file
146
.claude/specs/pool.md
Normal file
@@ -0,0 +1,146 @@
|
||||
# Connection pool
|
||||
|
||||
This is an architectural reference for `internal/pool/`. Read this before changing connection lifecycle, the wantConn queue, the conn state machine, or hooks. The pool is the single most common source of bugs in this codebase (`fix(pool):` is the top commit-prefix in the history) — almost always because someone violated an invariant below without realizing it.
|
||||
|
||||
This doc captures **invariants and the "why"**, not a tour of the code. File names anchor each section but the line numbers will drift.
|
||||
|
||||
---
|
||||
|
||||
## What the pool actually does
|
||||
|
||||
`ConnPool` (`internal/pool/pool.go`) hands out `*Conn` to callers, recycles them when returned, dials new ones when needed, and replaces them when they die. It's used by every client type (`Client`, `ClusterClient`, `Ring`, sentinel) — they all share the same pool implementation. There are two thin wrappers:
|
||||
|
||||
- `SingleConnPool` (`pool_single.go`) — wraps exactly one `*Conn`, no concurrency. Used during `initConn` (`HELLO`, `AUTH`, `CLIENT SETNAME`) and re-auth, where we must run commands on a specific connection that is *not* yet in the regular pool.
|
||||
- `StickyConnPool` (`pool_sticky.go`) — wraps a Pooler and pins one connection across many `Get`/`Put` cycles. Used by PubSub, where the subscription state lives on a specific TCP connection.
|
||||
|
||||
Concurrency from clients is bounded by `PoolSize` via a semaphore (`waitTurn`/`freeTurn`). `MaxActiveConns` is a hard ceiling that includes excess (non-pooled) connections; if 0, only `PoolSize` matters.
|
||||
|
||||
---
|
||||
|
||||
## The four primary invariants
|
||||
|
||||
### 1. Every `waitTurn` that succeeds must call `freeTurn`
|
||||
|
||||
The semaphore is the only thing keeping concurrency bounded. A leaked turn is permanently lost capacity. Every error path in `Get`, every retry loop, every panic-recovery must release. Audit new exit paths in `Get`/`Put` carefully — most pool bugs in history trace back to a missed `freeTurn`.
|
||||
|
||||
### 2. FIFO order on `wantConnQueue`
|
||||
|
||||
When the pool is exhausted, callers queue as `wantConn` requests in `wantConnQueue` (`want_conn.go`). They're served in arrival order. Two real bugs hinged on this:
|
||||
|
||||
- **#3777 — FIFO violation in `notifyWaiters`**: the wakeup loop re-read the atomic state on each iteration, so a goroutine that woke up fast could "skip the line" by re-entering `notifyWaiters` ahead of an earlier waiter. Fix: snapshot state locally inside `notifyWaiters` and only consider transitions made within this call. See the comment near `conn_state.go:300` ("atomic would let a fast goroutine's `Transition(StateIdle)` leak into our…").
|
||||
- **#3680 — zombie `wantConn` elements**: cancelled or timed-out waiters stayed in the queue head until the next `discardDoneAtFront`, which delayed real waiters and caused spurious retries. Fix: scan and prune done elements proactively.
|
||||
|
||||
If you change `wantConnQueue`, write a test that asserts a slow first waiter still gets the connection before a faster second waiter.
|
||||
|
||||
### 3. The `ConnState` machine is the single source of truth for liveness
|
||||
|
||||
`conn_state.go` defines six states:
|
||||
|
||||
```
|
||||
StateCreated → StateInitializing → StateIdle ⇄ StateInUse
|
||||
↓
|
||||
StateUnusable (handoff or re-auth)
|
||||
↓
|
||||
StateIdle / StateClosed
|
||||
```
|
||||
|
||||
- `StateCreated` — just allocated, no I/O yet.
|
||||
- `StateInitializing` — running `initConn` (`HELLO`, `AUTH`, etc.). Reachable only via `SingleConnPool`.
|
||||
- `StateIdle` — sitting in `idleConns`, eligible to be popped by `Get`.
|
||||
- `StateInUse` — handed out to a caller.
|
||||
- `StateUnusable` — owned by a background worker (handoff, re-auth). The pool will *not* return this conn from `Get` and will *not* close it from `Put`.
|
||||
- `StateClosed` — terminal.
|
||||
|
||||
There used to also be an atomic `Conn.closed` bool. It was removed in #3783 because it duplicated `StateClosed` and the duplication caused missed updates. **Do not re-introduce shadow flags.** If you need a new connection-level fact, add a state or use the existing fields under the state machine's lock.
|
||||
|
||||
`StateUnusable` is the contract that lets handoff and re-auth coexist with normal traffic without races. `OnGet` rejects unusable conns, `OnPut` keeps them in the pool (instead of recycling/closing), and the background worker drives them back to `StateIdle` (or removes them) when its work finishes.
|
||||
|
||||
### 4. Dial timeout is per-attempt, not cumulative
|
||||
|
||||
`DialerRetries` (default 5), `DialerRetryTimeout` (default 100ms), and the optional `DialerRetryBackoff` function shape retry behaviour. `DialTimeout` is applied **per attempt**: each retry gets a fresh window (#3705 — `fix(timeout): make dial timeout per-retry`). The caller's context deadline is honored only when it's tighter than the per-attempt budget. This prevents a long backoff from eating the next attempt's I/O time.
|
||||
|
||||
If all attempts fail, `tryDial` keeps probing in the background once a second so the pool can recover without a foreground request having to retry the whole sequence.
|
||||
|
||||
---
|
||||
|
||||
## Get / Put semantics
|
||||
|
||||
`Get`:
|
||||
1. `waitTurn` — acquire a semaphore token (blocks).
|
||||
2. `popIdle` — pop an idle conn, retrying past `StateUnusable`/`StateClosed` ones.
|
||||
3. If no idle conn, queue a `wantConn` and let `queuedNewConn` dial in a background goroutine.
|
||||
4. Run `OnGet` hooks (auth, handoff guard).
|
||||
5. Transition `StateIdle → StateInUse` and return.
|
||||
|
||||
`Put`:
|
||||
1. If the conn has unread bytes, peek for a RESP3 push notification. If it's a push frame, leave the buffered data in place (the push processor will consume it on the next `Get`); otherwise, treat the conn as broken and remove it.
|
||||
2. Run `OnPut` hooks. A hook may transition the conn to `StateUnusable` to schedule a background operation; if so, `Put` returns the conn to the pool **without** recycling so the worker can pick it up.
|
||||
3. If the conn is past `ConnMaxIdleTime` / `ConnMaxLifetime` (with jitter), close it.
|
||||
4. Otherwise transition `StateInUse → StateIdle` and prepend to `idleConns`.
|
||||
|
||||
The "unread data on Put → check for push frame" path is what makes RESP3 maintenance notifications work without dedicated reader goroutines. Don't simplify it.
|
||||
|
||||
### Errors on close are noise for stale conns
|
||||
|
||||
#3778 (`fix(pool): suppress pool Close() errors for stale connections`): a connection that exceeded `ConnMaxIdleTime` is half-broken by definition; reporting `EBADF`/`ECONNRESET` from its `Close` is alarm-fatigue. The pool only surfaces close errors for connections that were healthy when we decided to close them.
|
||||
|
||||
---
|
||||
|
||||
## `conn_check`: detecting dead sockets and push frames
|
||||
|
||||
`conn_check.go` (Unix) does a non-blocking `MSG_PEEK` on the socket to distinguish three situations on an idle connection:
|
||||
|
||||
- 0 bytes available, no error → healthy.
|
||||
- EOF → peer closed; remove the conn.
|
||||
- Bytes available → either a push notification (RESP3, leave it) or unsolicited data (broken protocol, remove it).
|
||||
|
||||
`conn_check_dummy.go` (non-Unix) is a no-op stub. Push notification handling on Windows is therefore best-effort; the `Put` peek-on-unread-data path remains correct cross-platform but is the only line of defense. If you change `conn_check`, check both files.
|
||||
|
||||
---
|
||||
|
||||
## Re-auth and handoff coexistence (#3547)
|
||||
|
||||
Both re-auth (driven by `internal/auth/streaming`) and handoff (driven by `maintnotifications`) need exclusive access to a connection while running on it. Both use `StateUnusable` to tell the pool "don't touch this." The bug fixed in #3547 was that re-auth could grab a connection that handoff had already targeted, causing the handoff worker to run against a connection whose credentials had changed.
|
||||
|
||||
The contract now: a conn that is `StateUnusable` *for handoff* must not be picked up by re-auth. Whichever subsystem marks the conn first wins; the other one observes the unusable state via `OnGet` and skips it. The auth pool hook lives in `internal/auth/streaming/pool_hook.go`; the handoff pool hook lives in `maintnotifications/pool_hook.go`. They share the pool's hook list — order of registration does not matter for correctness because the state-machine transitions are atomic, but be careful when adding a third subsystem that needs the same exclusion.
|
||||
|
||||
---
|
||||
|
||||
## Pool configuration
|
||||
|
||||
| Option | Default | Notes |
|
||||
|---|---|---|
|
||||
| `PoolSize` | `10 * GOMAXPROCS` | Soft cap on pooled conns. |
|
||||
| `MinIdleConns` | 0 | Pre-warmed at init; replenished after `Remove`. Background goroutine fills these via `dialHook`, hence the read-lock in `dialHook` (#3225). |
|
||||
| `MaxActiveConns` | 0 (unlimited) | Hard cap including excess (non-pooled) conns used during bursts. |
|
||||
| `PoolTimeout` | `ReadTimeout + 1s` | How long `waitTurn` will block. |
|
||||
| `ConnMaxIdleTime` | 30 min | Eviction in `Put`. |
|
||||
| `ConnMaxLifetime` | 0 (none) | Absolute age limit; checked on `Put`. |
|
||||
| `ConnMaxLifetimeJitter` | 0 | Random ±jitter on `ConnMaxLifetime` to avoid all conns expiring together (#3666). |
|
||||
| `ReadBufferSize` / `WriteBufferSize` | 32 KiB (since v9.12) | Was 4 KiB before. Sentinel client uses 4 KiB (#3476) because its connections are short-lived. If you see odd performance regressions, flip back to 4 KiB and compare. |
|
||||
|
||||
`ConnMaxLifetimeJitter` matters in deployments where many clients started at the same time: without jitter, all their connections expire in the same second and reconnect simultaneously, hammering the server (thundering herd).
|
||||
|
||||
---
|
||||
|
||||
## Hook integration (`hooks.go`)
|
||||
|
||||
`PoolHookManager` carries `OnGet`, `OnPut`, `OnRemove`. It's stored as an `atomic.Value` on `ConnPool` so reads in the hot path don't need a mutex. Hooks are called *outside* the pool's internal lock — never call back into the pool from inside a hook in a way that requires the lock you're not holding.
|
||||
|
||||
`OnGet` is allowed to reject a conn (e.g. handoff hook seeing `ShouldHandoff()` true). `OnPut` is allowed to mutate state (mark unusable, schedule background work). `OnRemove` is for cleanup (per-conn registries, listener unsubscribes).
|
||||
|
||||
When you add a new subsystem that needs per-connection state, prefer registering a pool hook over reaching into `ConnPool` directly. The hook is the seam.
|
||||
|
||||
---
|
||||
|
||||
## History worth knowing
|
||||
|
||||
The recurring shape of pool bugs:
|
||||
|
||||
- **State duplication**: shadow flags (`closed`, old `usable`/`Inited`/`used` atomics) drift out of sync with the real state. Removed in #3559 / #3783.
|
||||
- **Lost waiters / lost turns**: missing `freeTurn` on an error path, or `wantConn` left in the queue with no one to deliver to. #3680, #3777, #3626 (`putIdleConn` turn management).
|
||||
- **Race between background work and `Close`**: pool tries to close a conn that handoff/re-auth still owns. Fixed by `StateUnusable` + hook contract.
|
||||
- **Errors that aren't really errors**: stale-conn close errors (#3778), failed dial-tcp errors that should be treated as redirectable (#3786 — these are routing-layer, but the pool surfaces them).
|
||||
- **Identity verification (HELLO/SETINFO) failures coupling into pool init**: #3295, #3294, #3788. The pattern: feature handshakes during `initConn` should fail soft when the server doesn't support them, and must not deadlock or leave the conn half-initialized.
|
||||
|
||||
When you write a pool fix, add a regression test that fails without your change. The pool's behaviour is too easy to verify by inspection only.
|
||||
3
.gitignore
vendored
3
.gitignore
vendored
@@ -17,3 +17,6 @@ maintenanceNotifications/docs/
|
||||
# Docker-generated files (TLS certificates, cluster data, etc.)
|
||||
dockers/*/tls/
|
||||
dockers/osscluster-tls/
|
||||
|
||||
# Per-user Claude Code settings (machine/personal overrides, not shared policy)
|
||||
.claude/settings.local.json
|
||||
|
||||
228
AGENTS.md
Normal file
228
AGENTS.md
Normal file
@@ -0,0 +1,228 @@
|
||||
# AGENTS.md
|
||||
|
||||
Guidance for AI coding agents (and humans) working in this repository. This is
|
||||
the shared, tool-agnostic source of truth: Claude Code loads it through
|
||||
`CLAUDE.md` (which imports this file), and other agents (Codex, Cursor, Aider,
|
||||
Zed, …) read `AGENTS.md` directly. Edit repository guidance here, not in
|
||||
`CLAUDE.md`.
|
||||
|
||||
## Repository
|
||||
|
||||
go-redis is the official Redis client for Go. Module path:
|
||||
`github.com/redis/go-redis/v9` (Go 1.24+). The repo is a multi-module workspace
|
||||
— every directory containing a `go.mod` is built and tested independently:
|
||||
|
||||
- root (`github.com/redis/go-redis/v9`) — the client library.
|
||||
- `extra/redisotel`, `extra/redisotel-native`, `extra/redisprometheus`,
|
||||
`extra/rediscensus`, `extra/rediscmd` — instrumentation adapters with their
|
||||
own module paths (so they can pin large telemetry deps without forcing them on
|
||||
root consumers).
|
||||
- `internal/customvet` — custom `go vet` analyzers (also its own module).
|
||||
- `maintnotifications/e2e`, `doctests`, `fuzz`, examples under `example/` —
|
||||
separate modules.
|
||||
|
||||
The Makefile iterates over every `go.mod` (`GO_MOD_DIRS`) when running
|
||||
`test.ci`, `go_mod_tidy`, etc. When you add a dependency in one module, you
|
||||
almost never need to update the others.
|
||||
|
||||
## Common commands
|
||||
|
||||
Tests run against a Redis stack started via Docker Compose. Profiles in
|
||||
`docker-compose.yml` control which services come up (`standalone`, `cluster`,
|
||||
`sentinel`, `all`, `e2e`).
|
||||
|
||||
```sh
|
||||
make docker.start # bring up the full test stack (profile: all)
|
||||
make docker.stop
|
||||
make test # docker.start -> test.ci -> docker.stop
|
||||
make test.ci # run tests assuming containers are already up
|
||||
make test.ci.skip-vectorsets # when REDIS_VERSION < 8
|
||||
make bench # go test -bench=. (root module only)
|
||||
make fmt # gofumpt + goimports -local github.com/redis/go-redis
|
||||
make build
|
||||
make go_mod_tidy # go mod tidy across every module
|
||||
```
|
||||
|
||||
E2E (maintenance notifications) needs the extra `cae-resp-proxy` service:
|
||||
|
||||
```sh
|
||||
make test.e2e # starts e2e profile, runs ./maintnotifications/e2e/, tears down
|
||||
make test.e2e.docker # subset that runs inside docker
|
||||
make test.e2e.logic # logic-only tests, no proxy required
|
||||
```
|
||||
|
||||
Run a single test. The root suite is Ginkgo-based (`bsm/ginkgo` + `bsm/gomega`
|
||||
forks), so `go test -run` matches the Go-level wrapper and you focus a spec with
|
||||
the Ginkgo flag:
|
||||
|
||||
```sh
|
||||
go test -run TestGinkgoSuite . -ginkgo.focus="ZAdd"
|
||||
go test -run TestGinkgoSuite . -ginkgo.focus="cluster"
|
||||
```
|
||||
|
||||
Plain `go test` tests (most files outside the Ginkgo suite, e.g. `internal/...`,
|
||||
`maintnotifications/...`) work the usual way:
|
||||
|
||||
```sh
|
||||
go test -run TestConnStateMachine ./internal/pool/...
|
||||
go test -race -run TestCircuitBreaker ./maintnotifications/...
|
||||
```
|
||||
|
||||
Env knobs (passed through the Makefile):
|
||||
|
||||
- `REDIS_VERSION` — e.g. `8.8`. Drives both the test image tag and
|
||||
`main_test.go` version-gating (`SkipBeforeRedisVersion` /
|
||||
`SkipAfterRedisVersion`).
|
||||
- `CLIENT_LIBS_TEST_IMAGE` — full image ref, e.g.
|
||||
`redislabs/client-libs-test:8.8-m03`.
|
||||
- `RE_CLUSTER=true` — run against a Redis Enterprise cluster instead of the
|
||||
docker-compose stack (the suite then skips ring/sentinel/TLS-cluster setup).
|
||||
- `RCE_DOCKER=true` — Redis CE in docker (default for `make test`).
|
||||
- `REDIS_PORT` — override the default standalone port (`6380`).
|
||||
|
||||
CI also runs the custom vet tool:
|
||||
`go vet -vettool ./internal/customvet/customvet ./...`. The `setval` analyzer
|
||||
requires every `Cmder` with a `Result()` to also have a `SetVal()`.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Client types (root package)
|
||||
|
||||
All clients are in the root package and share most plumbing:
|
||||
|
||||
- `Client` (`redis.go`) — single-node client.
|
||||
- `ClusterClient` (`osscluster.go`) — Redis Cluster aware. `osscluster_router.go`
|
||||
routes commands to the right shard; `internal/routing/` handles cluster-wide
|
||||
aggregation policies (e.g. fan-out for `KEYS`, `DBSIZE`).
|
||||
- `Ring` (`ring.go`) — client-side sharding across independent Redis nodes
|
||||
(consistent hashing, no cluster protocol).
|
||||
- Failover client (`sentinel.go`) — Sentinel-managed failover.
|
||||
- `UniversalClient` (`universal.go`) — wrapper that picks one of the above based
|
||||
on options.
|
||||
|
||||
Command surface lives in topical files: `string_commands.go`, `hash_commands.go`,
|
||||
`stream_commands.go`, `search_commands.go`, `vectorset_commands.go`, etc. Each
|
||||
file defines methods on the shared `Cmdable` interface so every client type gets
|
||||
the same API.
|
||||
|
||||
### Hooks (`redis.go` `hooksMixin`)
|
||||
|
||||
Three hook chains run around every operation: `DialHook`, `ProcessHook`,
|
||||
`ProcessPipelineHook`. Hooks are registered via `client.AddHook(...)` and chain
|
||||
in FIFO order; each hook must call `next` to continue. When a hook wraps an
|
||||
error, it must call `cmd.SetErr(wrappedErr)` so the typed-error helpers
|
||||
(`redis.IsLoadingError`, `IsMovedError`, etc. in `error.go`) keep working through
|
||||
`errors.As`. The README has a longer pipeline-hook example.
|
||||
|
||||
### Connection pool (`internal/pool`)
|
||||
|
||||
Owns dialing, idle/active connection bookkeeping, conn state (`conn_state.go`),
|
||||
pubsub-conn lifecycle (`pubsub.go`), and the dial-retry/backoff logic that powers
|
||||
`DialerRetries` / `DialerRetryBackoff` (also exposed at `dial_retry_backoff.go`
|
||||
in the root). `OnConnect`, `MinIdleConns`, and the buffer-size options
|
||||
(`ReadBufferSize`/`WriteBufferSize`, default 32 KiB since v9.12) flow through
|
||||
here.
|
||||
|
||||
### Protocol (`internal/proto`)
|
||||
|
||||
RESP2/RESP3 reader and writer. Push notifications (RESP3 `>`-prefixed frames) are
|
||||
peeked here and dispatched via the `push/` package. The `push.Registry` lets
|
||||
callers register handlers for specific notification names;
|
||||
`maintnotifications/push_notification_handler.go` is how `maintnotifications`
|
||||
plugs in.
|
||||
|
||||
### Maintenance notifications (`maintnotifications/`)
|
||||
|
||||
This is a non-trivial subsystem worth understanding before touching
|
||||
cluster/handoff code. It listens for RESP3 push notifications about cluster
|
||||
maintenance (`MOVING`, `MIGRATING`, `MIGRATED`, `FAILING_OVER`, `FAILED_OVER`
|
||||
for standalone; `SMIGRATING`, `SMIGRATED` for cluster) and performs seamless
|
||||
connection handoff to new endpoints. Key pieces:
|
||||
|
||||
- `manager.go` — coordinates state transitions.
|
||||
- `handoff_worker.go` — moves in-flight ops to new connections.
|
||||
- `pool_hook.go` — integrates with `internal/pool` to mark/replace connections.
|
||||
- `circuit_breaker.go` — backs off when the upstream is unhealthy.
|
||||
- `state.go` — per-connection state machine.
|
||||
- E2E coverage lives in `maintnotifications/e2e/` and drives a fault-injector /
|
||||
RESP proxy (`cae-resp-proxy`).
|
||||
|
||||
Configuration is via `redis.Options.MaintNotificationsConfig`; modes are
|
||||
`ModeAuto` (default), `ModeEnabled` (require server support), `ModeDisabled`.
|
||||
RESP3 (`Protocol: 3`) is required.
|
||||
|
||||
### Authentication (`auth/`, `internal/auth/streaming`)
|
||||
|
||||
Four credential sources, in priority order: streaming provider (e.g. Entra ID
|
||||
via `go-redis-entraid`), context-based provider, function provider, static
|
||||
`Username`/`Password`. The streaming provider is what enables token rotation
|
||||
without reconnecting — the listener in `auth/reauth_credentials_listener.go`
|
||||
issues `AUTH` on each refresh.
|
||||
|
||||
### Internal helpers
|
||||
|
||||
- `internal/hscan` — struct scanning for `HGETALL` results (`Scan` interface
|
||||
re-exported as `redis.Scanner`).
|
||||
- `internal/hashtag` — extracts `{tag}` segments for cluster slot routing.
|
||||
- `internal/routing` — aggregator policies and shard pickers used by
|
||||
`ClusterClient` for multi-shard commands.
|
||||
- `internal/otel` — small OpenTelemetry shim used to keep root free of telemetry
|
||||
deps; full instrumentation lives in `extra/redisotel-native`.
|
||||
|
||||
## Architectural specs
|
||||
|
||||
Read the relevant design doc **before** changing code in that subsystem. They
|
||||
cover invariants and decisions that aren't obvious from the code, and are plain
|
||||
markdown any tool or editor can open:
|
||||
|
||||
- `.claude/specs/pool.md` — connection pool: `wantConn` queue and FIFO
|
||||
discipline, `ConnState` machine, dial retry/backoff, hook integration, the
|
||||
re-auth/handoff coexistence contract.
|
||||
- `.claude/specs/cluster-routing.md` — slot computation, MOVED/ASK redirection,
|
||||
request/response policies, aggregators, replica routing, topology reload,
|
||||
cross-slot rules.
|
||||
- `.claude/specs/maintnotifications.md` — RESP3 push notification protocol, mode
|
||||
handshake, per-conn state, handoff worker pool, circuit breaker, endpoint-type
|
||||
resolution, cluster vs. standalone differences.
|
||||
|
||||
## Conventions
|
||||
|
||||
- New `Cmder` type → also implement `SetVal` (the custom vet `setval` check
|
||||
enforces this; `SetErr` is on the embedded `baseCmd`).
|
||||
- Wrap errors with custom error types that implement `Unwrap`, or use
|
||||
`fmt.Errorf("...: %w", err)`. Always call `cmd.SetErr(...)` after wrapping so
|
||||
typed-error checks still pass.
|
||||
- `gofumpt` + `goimports -local github.com/redis/go-redis` is the formatter
|
||||
(`make fmt`); CI runs both.
|
||||
- Don't log directly — use `internal.Logger` (set via `redis.SetLogger`);
|
||||
`logging.Disable()` is called in tests.
|
||||
- Version-gate Redis-version-specific tests with `SkipBeforeRedisVersion` /
|
||||
`SkipAfterRedisVersion` rather than skipping at the suite level.
|
||||
|
||||
### Commits and PRs
|
||||
|
||||
Conventional Commits, short and exact — `<type>(<scope>): <imperative summary>`.
|
||||
Subject ≤50 chars (hard cap 72), imperative ("add", not "added"), no trailing
|
||||
period. Body only when the *why* isn't obvious from the diff; wrap at 72.
|
||||
|
||||
- Types: `feat`, `fix`, `refactor`, `perf`, `docs`, `test`, `chore` (also
|
||||
`build`, `ci`, `style`, `revert`).
|
||||
- Scope = the subsystem touched, lowercase: `pool`, `conn`, `pubsub`,
|
||||
`sentinel`, `retry`, `command`/`cmd`, `vectorset`, `otel`, `streams`, `push`,
|
||||
`deps`, `ci`, `tests`, `docs`. Omit only for genuinely cross-cutting changes.
|
||||
- Breaking change: `feat(scope)!: ...` plus a `BREAKING CHANGE:` body line.
|
||||
Reference issues/PRs at the end — `Closes #42`, `Refs #17`.
|
||||
- **No AI-attribution trailer.** Do not add `Co-Authored-By: …`, "Generated with
|
||||
…", or any AI-attribution line to commits or PR bodies in this repo.
|
||||
|
||||
## Repo-specific tooling
|
||||
|
||||
`.claude/` holds shared AI config:
|
||||
|
||||
- `commands/` — slash commands (e.g. `check-ci`, which summarizes a PR's CI).
|
||||
- `skills/` — task playbooks: `testing`, `add-command`, `commit-style`,
|
||||
`update-ci-image`, `prepare-release`.
|
||||
- `specs/` — the architecture docs listed above.
|
||||
|
||||
For Claude Code, the skills auto-trigger from their descriptions. For other
|
||||
tools, each `SKILL.md` is plain markdown you can open and follow directly.
|
||||
19
CLAUDE.md
Normal file
19
CLAUDE.md
Normal file
@@ -0,0 +1,19 @@
|
||||
# CLAUDE.md
|
||||
|
||||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
|
||||
|
||||
Repository guidance — layout, commands, architecture, and conventions — is
|
||||
maintained in [`AGENTS.md`](AGENTS.md) so it stays shared across AI tools. It is
|
||||
imported below; edit `AGENTS.md`, not this file, for that content.
|
||||
|
||||
@AGENTS.md
|
||||
|
||||
## Claude-specific
|
||||
|
||||
- Repo-local **skills** under `.claude/skills/` auto-trigger from their
|
||||
descriptions — no need to invoke them manually (the set is listed in
|
||||
`AGENTS.md`).
|
||||
- **Slash commands** under `.claude/commands/` (e.g. `/check-ci`) are available
|
||||
in-session.
|
||||
- Architectural **specs** under `.claude/specs/` are read on demand; open the
|
||||
relevant one before changing that subsystem.
|
||||
Reference in New Issue
Block a user