{"owner":"vitessio","repo":"vitess","hasSkills":true,"hasMcp":false,"mcpConfig":null,"found":["CLAUDE.md",".github/copilot-instructions.md"],"skills":{"CLAUDE.md":"## :handshake: Our Partnership\n\n**We're building this together.** You're not just executing tasks - you're helping design and implement the best possible solution. This means:\n\n- Challenge my suggestions when something feels wrong\n- Ask me to explain my reasoning\n- Propose alternative approaches\n- Take time to think through problems\n\n**Quality is non-negotiable.** We'd rather spend an hour designing than 3 hours fixing a rushed implementation.\n\n## :thought_balloon: Before We Code\n\nAlways discuss first:\n- What problem are we solving?\n- What's the ideal solution?\n- What tests would prove it works?\n- Are we making the codebase better?\n\n## Strict Task Adherence\n\n**Only do exactly what I ask for - nothing more, nothing less.**\n\n- Do NOT add explanatory comments unless asked\n- Do NOT make \"improvements\" or \"clean up\" code beyond the specific task\n- Do NOT add features, optimizations, or enhancements I didn't mention\n- If there is something you think should be done, suggest it, but don't do it until asked to\n\n**Red flags that indicate you're going beyond the task:**\n- \"Let me also...\"\n- \"While I'm at it...\"\n- \"I should also update...\"\n- \"Let me improve...\"\n- \"I'll also clean up...\"\n\n**If the task is complete, STOP. Don't look for more work to do.**\n\n## :test_tube: Test-Driven Development\n\nTDD isn't optional - it's how we ensure quality:\n\n### The TDD Cycle\n1. **Red** - Write a failing test that defines success\n2. **Green** - Write minimal code to pass\n3. **Refactor** - Make it clean and elegant\n\n### Example TDD Session\n```go\n// Step 1: Write the test first\nfunc TestConnectionBilateralCleanup(t *testing.T) {\n    // Define what success looks like\n    client, server := testutils.CreateConnectedTCPPair()\n    \n    // Test the behavior we want\n    client.Close()\n    \n    // Both sides should be closed\n    assert.Eventually(t, func() bool {\n        return isConnectionClosed(server)\n    })\n}\n\n// Step 2: See it fail (confirms we're testing the right thing)\n// Step 3: Implement the feature\n// Step 4: See it pass\n// Step 5: Refactor for clarity\n```\n\nTo make sure tests are easy to read, we use `github.com/stretchr/testify/assert` and `github.com/stretchr/testify/require` for assertions:\n- Use `require` (not `assert`) when the test cannot continue after a failure (e.g., `require.NoError` after setup that must succeed)\n- Use `assert.Eventually` instead of manual `time.Sleep()` and timeouts\n- Use `t.Context()` instead of `context.Background()` — it integrates with test cancellation\n- Use `t.Cleanup()` for test teardown\n- Use `assert.ErrorContains` / `require.ErrorContains` to check error messages\n- Use the `_test.go` suffix for mocks and test helpers that are only used by the current package's tests; if helpers or mocks need to be imported by other packages' tests or fuzz harnesses, put them in a normal reusable package such as `testlib` or `testutil`\n- CI timeouts must be generous (30s+) — GitHub Actions runners can be resource-starved with multi-second pauses; sub-second timeouts cause flakiness with no recourse but retry\n- Do not use t.Fatal or t.Error in tests, but instead require and assert\n\n### Test Honesty\n- A test must actually exercise the condition its name and doc claim, must fail on `main` without the fix it guards, and must not duplicate coverage that a unit test already pins down precisely. Tests that pass identically with or without the fix waste CI time and create false confidence.\n\n## :rotating_light: Error Handling Excellence\n\nError handling is not an afterthought - it's core to reliable software.\n\n### Go Error Patterns\n```go\n// YES - Clear error context with vterrors\nfunc ProcessUser(id string) (*User, error) {\n    if id == \"\" {\n        return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, \"user ID cannot be empty\")\n    }\n\n    user, err := db.GetUser(id)\n    if err != nil {\n        return nil, vterrors.Wrapf(err, \"failed to get user %s\", id)\n    }\n\n    return user, nil\n}\n\n// NO - Swallowing errors\nfunc ProcessUser(id string) *User {\n    user, _ := db.GetUser(id)  // What if this fails?\n    return user\n}\n```\n\n### Error Handling Principles\n1. **Use `vterrors`** - Prefer `vterrors` over `fmt.Errorf` or `errors` package, with an appropriate `vtrpcpb.Code` (e.g., `vtrpcpb.Code_FAILED_PRECONDITION` for unexpected input values, `vtrpcpb.Code_INTERNAL` for internal operation failures)\n2. **Wrap errors with context** - Use `vterrors.Wrapf(err, \"context\")`\n3. **Validate early** - Check inputs before doing work\n4. **Fail fast** - Don't continue with invalid state\n5. **Log appropriately** - Errors at boundaries, debug info internally\n6. **Return structured errors** - Use error types for different handling\n7. **Never silently swallow errors** - When recovering from an error (e.g., restarting replication), always log the original error before the recovery attempt so operators can trace what happened\n8. **Log with context** - Include workflow name, recovery type, tablet alias, and other identifiers in log messages — a keyspace/tablet can have many concurrent workflows\n\n### Failure-Path Safety\nMulti-step operations must not leave the system in a half-applied state:\n- If step 2 of 3 succeeds but step 3 fails, ensure step 2 is rolled back or the cleanup path handles it\n- Deferred cleanup must use bounded contexts — never `context.Background()` for operations that could hang indefinitely\n- When holding a mutex, always bound the operation with a reasonable timeout\n- Test failure paths, not just the happy path — a test that only proves \"step X ran\" without covering \"what if step X+1 fails\" is incomplete\n\n### Testing Error Paths\n```go\nfunc TestProcessUser_InvalidID(t *testing.T) {\n    _, err := ProcessUser(\"\")\n    assert.ErrorContains(t, err, \"cannot be empty\")\n}\n\nfunc TestProcessUser_DatabaseError(t *testing.T) {\n    mockDB.EXPECT().GetUser(\"123\").Return(nil, errors.New(\"db connection failed\"))\n    \n    _, err := ProcessUser(\"123\")\n    assert.ErrorContains(t, err, \"failed to get user\")\n}\n```\n\n## :triangular_ruler: Design Principles\n\n### 1. Simple is Better Than Clever\n```go\n// YES - Clear and obvious\nif user.NeedsMigration() {\n    return migrate(user)\n}\n\n// NO - Clever but unclear\nreturn user.NeedsMigration() && migrate(user) || user\n```\n\n### 2. Explicit is Better Than Implicit\n- Clear function names\n- Obvious parameter types\n- No hidden side effects\n\n### 3. Conditions Must Be as Specific as the Intent\n- Guard clauses should match *exactly* the intended case, not just currently-known cases\n- A check like `len(tables) == 0` when you mean \"is virtual dual\" will silently fire for future zero-table cases\n- A nil return from a helper (e.g., `operatorKeyspace()` returning nil for composite operators) should not be treated as \"safe to skip\" — handle it explicitly\n- When catching MySQL error codes, match the specific codes that apply to your call site, not a broad class\n\n### 4. Zero-Value / Default Behavior Safety\n- New struct fields must not change behavior for existing callers who omit them\n- Prefer negative-polarity booleans (`PreventCrossKeyspaceReads` not `AllowCrossKeyspaceReads`) so the zero-value preserves existing behavior\n- When a flag value of `0` or empty previously meant \"disabled,\" don't change it to mean \"unlimited\" — preserve the existing semantic or make the change explicit\n- Validate mutually exclusive flags in `PreRunE` and add unit tests for invalid combinations\n\n### 5. Performance with Clarity\n- Optimize hot paths, but keep code readable\n- Preallocate slices and maps when the size is known: `make([]T, 0, len(source))`\n- Avoid duplicate work — cache results of expensive calls like `reflect.Value.MapKeys()` instead of calling twice\n- Document why, not what\n\n### 6. Fail Fast and Clearly\n- Validate inputs early\n- Return clear error messages\n- Help future debugging\n\n### 7. Interfaces Define What You Need, Not What You Provide\n- When you need something from another component, define the interface in your package\n- Don't look at what someone else provides - define exactly what you require\n- This keeps interfaces small, focused, and prevents unnecessary coupling\n- Types and their methods live together. At the top of files, use a single ```type ()``` with all type declarations inside.\n\n### 8. Go-Specific Best Practices\n- **Receiver naming** - Use consistent, short receiver names (e.g., `u *User`, not `user *User`)\n- **Package naming** - Short, descriptive, lowercase without underscores\n- **Interface naming** - Single-method interfaces end in `-er` (Reader, Writer, Handler)\n- **Context first** - Always pass `context.Context` as the first parameter\n- **Context cancellation** - Prefer `context.WithoutCancel(ctx)` over `context.Background()` when you need a non-cancellable context but still want to preserve context values (tracing, caller ID)\n- **Channels for coordination** - Use channels to coordinate goroutines, not shared memory\n- **No naked returns in non-trivial functions** - For functions with named return values, avoid bare `return` and explicitly return all result values (very small helpers are the only exception). This does not prohibit plain `return` in `func f() { ... }` when used for early-exit/guard clauses.\n- **Reduce nesting** - Prefer early returns and guard clauses over deeply nested `if` conditions\n- **Copyright header** - New Go files must include the project copyright header with the current year\n- **Always run `scripts/fmt <changed-go-files>`** before committing - this is mandatory\n- **Use format verbs precisely** - Use `%s` for strings and `%d` for integers, not `%v` for everything\n- **Structured logging** - New log messages should use structured logging with `slog`-style fields (e.g., `log.Warn(\"message\", slog.Any(\"error\", err))`) rather than printf-style logging with format strings\n- **Reuse existing helpers** - Before writing new parsing/validation code, check for existing utilities (e.g., `sqlerror` package for MySQL error codes, `mysqlctl.ParseVersionString()`, `strings.Split()`, `topoproto.TabletAliasString()` for formatting tablet aliases)\n\n## :building_construction: Vitess-Specific Conventions\n\n### Generated Code\n- **Never** directly edit files with a `Code generated ... DO NOT EDIT` header - these are generated and will be overwritten\n- Run `make codegen` to regenerate after modifying source definitions\n\n#### Protobufs\n- **Never** directly edit files under `go/vt/proto/` - they are generated from `proto/*.proto` protobuf definitions\n- After modifying `proto/*.proto` files, run `make proto` to regenerate\n- Avoid storing timestamps or time durations as integers; use `vttime.Time` for timestamps and `vttime.Duration` (or `google.protobuf.Duration`, as appropriate) for durations instead\n- Avoid storing tablet aliases as a string: use `topodata.TabletAlias`\n\n#### SQL Parser\n- **Never** directly edit these generated files in `go/vt/sqlparser/`: `sql.go`, `ast_clone.go`, `ast_copy_on_rewrite.go`, `ast_equals.go`, `ast_format_fast.go`, `ast_path.go`, `ast_rewrite.go`, `ast_visit.go`, `cached_size.go`\n- After modifying source files (e.g., `sql.y`, AST definitions), run `make codegen` to regenerate\n- Field order in AST structs matters — generated walkers visit fields in declaration order, so reordering fields changes semantic-analysis walk order and can break scope setup\n\n### Command-Line Flags\n- New flags must **not** use underscores (use hyphens instead)\n- When flags are added or modified, update the corresponding `go/flags/endtoend/` files - column/whitespace alignment matters\n\n### TabletAlias Formatting\n- Format `*topodatapb.TabletAlias` using `topoproto.TabletAliasString(alias)` in logs and error messages so that tablet aliases are human-readable\n\n### MySQL Flavor Isolation\n- MySQL-version-specific behavior belongs in the corresponding flavor implementation (e.g., MariaDB handling in the MariaDB flavor file), not in generic code\n- Be aware that MariaDB and older MySQL versions may not support all system variables (e.g., `super_read_only`) — other Vitess call sites already warn-and-continue for `ERUnknownSystemVariable`\n\n### User-Visible Changes\n- Any user-visible behavioral change — even a correctness fix — needs explicit callout in release/deployment notes\n- Removing or renaming a public API function (e.g., in `sqlparser`) is a breaking change for downstream users — call it out explicitly or keep a thin compatibility wrapper\n- Changelog summaries are for key changes all users should know about — internal implementation details don't belong there\n- Keep PRs clean of unrelated diffs (e.g., stray `package-lock.json` changes, `go.sum` without `go mod tidy`)\n\n### Release Cycle & Compatibility\nVitess ships a major release roughly every 6 months, each supported for 12 months (see the [release cycle](https://vitess.io/docs/releases/release-cycle/) docs and `doc/internal/release/versioning.md`). Some changes therefore take 1-3 release cycles to complete — recognize when a task needs multi-release staging and propose the staged plan instead of doing it all in one release.\n\n- **Backwards AND forwards compatibility** must hold between consecutive major versions ([VEP-1](https://github.com/vitessio/enhancements/blob/main/veps/vep-1.md)) — clusters run mixed-version components during rolling upgrades, and downgrade by one major version must work too. CI enforces this via the `upgrade_downgrade_test_*` workflows (the `_next_release` variants test against the next major)\n- **Deprecation is a multi-release cycle** (minimum; maintainers may extend it):\n  - Behavior changes: release N announces + warns (default unchanged, opt-in flag), release N+1 may flip the default (old behavior restorable via the now-deprecated flag), release N+2 removes the flag/old behavior\n  - Simple removals (obsolete utilities, flags): warn in release N, remove in release N+1\n  - Never remove or change a default in the same release that introduces the deprecation warning\n- **Protobuf changes must be wire-compatible in both directions**: never renumber, retype, or reuse removed field numbers; new fields must tolerate being absent (an older peer never sends them) and their zero value must preserve existing behavior. The VTGate RPC protos are public API per `doc/internal/release/versioning.md`\n- **Data written by a live system** (topology data, Vitess-internal tables, on-disk formats) is covered by the same promise — a change that breaks the upgrade *or downgrade* path of a running cluster is a breaking change even if the data is \"internal\"\n- Experimental features are excluded from the compatibility promise and the deprecation rules\n\n## :mag: Debugging & Troubleshooting\n\nWhen things don't work as expected, we debug systematically:\n\n### Debugging Strategy\n1. **Reproduce reliably** - Create a minimal failing case\n2. **Isolate the problem** - Binary search through the system\n3. **Understand the data flow** - Trace inputs and outputs\n4. **Question assumptions** - What did we assume was working?\n5. **Fix the root cause** - Not just the symptoms\n\n### Debugging Tools & Techniques\n```go\n// Use structured logging (slog-style) for new code\nlog.Info(\"Starting payment processing\",\n    slog.String(\"user_id\", userID),\n    slog.String(\"action\", \"process_payment\"),\n    slog.Float64(\"amount\", amount),\n)\n\n// Add strategic debug points\nfunc processPayment(amount float64) error {\n    if amount <= 0 {\n        return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, \"invalid amount: %f\", amount)\n    }\n\n    // More processing...\n    log.Info(\"Payment validation passed\")\n    return nil\n}\n```\n\n### When Stuck\n- Write a test that reproduces the issue\n- Add logging to understand data flow  \n- Use the debugger to step through code\n- Rubber duck explain the problem\n- Take a break and come back fresh\n\n## :recycle: Refactoring Legacy Code\n\nWhen improving existing code, we move carefully and systematically:\n\n### Refactoring Strategy\n1. **Understand first** - Read and comprehend the existing code\n2. **Add tests** - Create safety nets before changing anything  \n3. **Small steps** - Make tiny, verifiable improvements\n4. **Preserve behavior** - Keep the same external interface\n5. **Measure improvement** - Verify it's actually better\n\n### Safe Refactoring Process\n```go\n// Step 1: Add characterization tests\nfunc TestLegacyProcessor_ExistingBehavior(t *testing.T) {\n    processor := &LegacyProcessor{}\n    \n    // Document current behavior, even if it seems wrong\n    result := processor.Process(\"input\")\n    assert.Equal(t, \"weird_legacy_output\", result)\n}\n\n// Step 2: Refactor with tests passing\nfunc (p *LegacyProcessor) Process(input string) string {\n    // Improved implementation that maintains the same behavior\n    return processWithNewLogic(input)\n}\n\n// Step 3: Now we can safely change the behavior\nfunc TestProcessor_ImprovedBehavior(t *testing.T) {\n    processor := &Processor{}\n    \n    result := processor.Process(\"input\")\n    assert.Equal(t, \"expected_output\", result)\n}\n```\n\n## :arrows_counterclockwise: Development Workflow\n\n### Starting a Feature\n1. **Discuss** - \"I'm thinking about implementing X. Here's my approach...\"\n2. **Design** - Sketch out the API and key components\n3. **Test** - Write tests that define the behavior\n4. **Implement** - Make the tests pass\n5. **Review** - \"Does this make sense? Any concerns?\"\n\n### Making Changes\n1. **Small PRs** - Easier to review and less risky\n2. **Incremental** - Build features piece by piece\n3. **Always tested** - No exceptions\n4. **Clear commits** - Each commit should have a clear purpose\n\n### Git and PR Workflow\n\n**CRITICAL: Git commands are ONLY for reading state - NEVER for modifying it.**\n- **NEVER** use git commands that modify the filesystem unless explicitly told to commit\n- You may read git state: `git status`, `git log`, `git diff`, `git branch --show-current`\n- You may NOT: `git commit`, `git add`, `git reset`, `git checkout`, `git restore`, `git rebase`, `git push`, etc.\n- **ONLY commit when explicitly asked to commit**\n- Always sign git commits with the `git commit --signoff` flag\n- When asked to commit, do it once and stop\n- Only I can modify git state unless you've been given explicit permission to commit\n\n**Once a PR is created, NEVER amend commits or rewrite history.**\n- Always create new commits after PR is created\n- No `git commit --amend` after pushing to a PR branch\n- No `git rebase` that rewrites commits in the PR\n- No force pushes to PR branches\n- This keeps the PR history clean and reviewable\n\n**When asked to write a PR description:**\n1. **Use `gh` CLI** - Always use `gh pr edit <number>` to update PRs\n2. **Update both body and title** - Use `--body` and `--title` flags\n3. **Be informal, humble, and short** - Keep it conversational and to the point\n4. **Credit appropriately** - If Claude Code wrote most of it, mention that\n5. **Example format**:\n   ```\n   ## What's this?\n   [Brief explanation of the feature/fix]\n\n   ## How it works\n   [Key implementation details]\n\n   ## Usage\n   [Code examples if relevant]\n\n   ---\n   _Most of this was written by Claude Code - I just provided direction._\n   ```\n\n## :memo: Code Review Mindset\n\nWhen reviewing code (yours or mine), ask:\n- Is this the simplest solution?\n- Will this make sense in 6 months?\n- Are edge cases handled?\n- Is it well tested?\n- Does it improve the codebase?\n\n## :dart: Common Patterns\n\n### Feature Implementation\n```\nYou: \"Let's add feature X\"\nMe: \"Sounds good! What's the API going to look like? What are the main use cases?\"\n[Discussion of design]\nMe: \"Let me write some tests to clarify the behavior we want\"\n[TDD implementation]\nMe: \"Here's what I've got. What do you think?\"\n```\n\n### Bug Fixing\n```\nYou: \"We have a bug where X happens\"\nMe: \"Let's write a test that reproduces it first\"\n[Test that fails]\nMe: \"Great, now we know exactly what we're fixing\"\n[Fix implementation]\n```\n\n### Performance Work\n```\nYou: \"This seems slow\"\nMe: \"Let's benchmark it first to get a baseline\"\n[Benchmark results]\nMe: \"Now let's optimize without breaking functionality\"\n[Optimization with tests passing]\n```\n\n## :rocket: Shipping Quality\n\nBefore considering any work \"done\":\n- [ ] Tests pass and cover the feature\n- [ ] Code is clean and readable\n    - [ ] Golang code passes `scripts/fmt <changed-go-files>`\n- [ ] Edge cases are handled\n- [ ] Performance is acceptable\n- [ ] Documentation is updated if needed\n- [ ] We're both happy with it\n\nRemember: We're crafting software, not just making it work. Every line of code is an opportunity to make the system better.\n",".github/copilot-instructions.md":"# Code Review Instructions\n\n## Priority\nOnly comment on issues that affect correctness, security, or performance.\nDo NOT comment on: style preferences, minor naming conventions, formatting, or\nissues already enforced by our linter/CI pipeline.\n\n## Confidence threshold\nOnly leave a comment when you have HIGH CONFIDENCE (>80%) that a real problem exists.\nDo not flag potential issues speculatively.\n\n## Severity filter\nSkip LOW severity issues entirely. Focus on HIGH and CRITICAL issues only.\n\n## CI context\nDo not flag issues that are caught by our automated CI/CD pipeline (linting, tests, type checks).\n"},"files":{"CLAUDE.md":"## :handshake: Our Partnership\n\n**We're building this together.** You're not just executing tasks - you're helping design and implement the best possible solution. This means:\n\n- Challenge my suggestions when something feels wrong\n- Ask me to explain my reasoning\n- Propose alternative approaches\n- Take time to think through problems\n\n**Quality is non-negotiable.** We'd rather spend an hour designing than 3 hours fixing a rushed implementation.\n\n## :thought_balloon: Before We Code\n\nAlways discuss first:\n- What problem are we solving?\n- What's the ideal solution?\n- What tests would prove it works?\n- Are we making the codebase better?\n\n## Strict Task Adherence\n\n**Only do exactly what I ask for - nothing more, nothing less.**\n\n- Do NOT add explanatory comments unless asked\n- Do NOT make \"improvements\" or \"clean up\" code beyond the specific task\n- Do NOT add features, optimizations, or enhancements I didn't mention\n- If there is something you think should be done, suggest it, but don't do it until asked to\n\n**Red flags that indicate you're going beyond the task:**\n- \"Let me also...\"\n- \"While I'm at it...\"\n- \"I should also update...\"\n- \"Let me improve...\"\n- \"I'll also clean up...\"\n\n**If the task is complete, STOP. Don't look for more work to do.**\n\n## :test_tube: Test-Driven Development\n\nTDD isn't optional - it's how we ensure quality:\n\n### The TDD Cycle\n1. **Red** - Write a failing test that defines success\n2. **Green** - Write minimal code to pass\n3. **Refactor** - Make it clean and elegant\n\n### Example TDD Session\n```go\n// Step 1: Write the test first\nfunc TestConnectionBilateralCleanup(t *testing.T) {\n    // Define what success looks like\n    client, server := testutils.CreateConnectedTCPPair()\n    \n    // Test the behavior we want\n    client.Close()\n    \n    // Both sides should be closed\n    assert.Eventually(t, func() bool {\n        return isConnectionClosed(server)\n    })\n}\n\n// Step 2: See it fail (confirms we're testing the right thing)\n// Step 3: Implement the feature\n// Step 4: See it pass\n// Step 5: Refactor for clarity\n```\n\nTo make sure tests are easy to read, we use `github.com/stretchr/testify/assert` and `github.com/stretchr/testify/require` for assertions:\n- Use `require` (not `assert`) when the test cannot continue after a failure (e.g., `require.NoError` after setup that must succeed)\n- Use `assert.Eventually` instead of manual `time.Sleep()` and timeouts\n- Use `t.Context()` instead of `context.Background()` — it integrates with test cancellation\n- Use `t.Cleanup()` for test teardown\n- Use `assert.ErrorContains` / `require.ErrorContains` to check error messages\n- Use the `_test.go` suffix for mocks and test helpers that are only used by the current package's tests; if helpers or mocks need to be imported by other packages' tests or fuzz harnesses, put them in a normal reusable package such as `testlib` or `testutil`\n- CI timeouts must be generous (30s+) — GitHub Actions runners can be resource-starved with multi-second pauses; sub-second timeouts cause flakiness with no recourse but retry\n- Do not use t.Fatal or t.Error in tests, but instead require and assert\n\n### Test Honesty\n- A test must actually exercise the condition its name and doc claim, must fail on `main` without the fix it guards, and must not duplicate coverage that a unit test already pins down precisely. Tests that pass identically with or without the fix waste CI time and create false confidence.\n\n## :rotating_light: Error Handling Excellence\n\nError handling is not an afterthought - it's core to reliable software.\n\n### Go Error Patterns\n```go\n// YES - Clear error context with vterrors\nfunc ProcessUser(id string) (*User, error) {\n    if id == \"\" {\n        return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, \"user ID cannot be empty\")\n    }\n\n    user, err := db.GetUser(id)\n    if err != nil {\n        return nil, vterrors.Wrapf(err, \"failed to get user %s\", id)\n    }\n\n    return user, nil\n}\n\n// NO - Swallowing errors\nfunc ProcessUser(id string) *User {\n    user, _ := db.GetUser(id)  // What if this fails?\n    return user\n}\n```\n\n### Error Handling Principles\n1. **Use `vterrors`** - Prefer `vterrors` over `fmt.Errorf` or `errors` package, with an appropriate `vtrpcpb.Code` (e.g., `vtrpcpb.Code_FAILED_PRECONDITION` for unexpected input values, `vtrpcpb.Code_INTERNAL` for internal operation failures)\n2. **Wrap errors with context** - Use `vterrors.Wrapf(err, \"context\")`\n3. **Validate early** - Check inputs before doing work\n4. **Fail fast** - Don't continue with invalid state\n5. **Log appropriately** - Errors at boundaries, debug info internally\n6. **Return structured errors** - Use error types for different handling\n7. **Never silently swallow errors** - When recovering from an error (e.g., restarting replication), always log the original error before the recovery attempt so operators can trace what happened\n8. **Log with context** - Include workflow name, recovery type, tablet alias, and other identifiers in log messages — a keyspace/tablet can have many concurrent workflows\n\n### Failure-Path Safety\nMulti-step operations must not leave the system in a half-applied state:\n- If step 2 of 3 succeeds but step 3 fails, ensure step 2 is rolled back or the cleanup path handles it\n- Deferred cleanup must use bounded contexts — never `context.Background()` for operations that could hang indefinitely\n- When holding a mutex, always bound the operation with a reasonable timeout\n- Test failure paths, not just the happy path — a test that only proves \"step X ran\" without covering \"what if step X+1 fails\" is incomplete\n\n### Testing Error Paths\n```go\nfunc TestProcessUser_InvalidID(t *testing.T) {\n    _, err := ProcessUser(\"\")\n    assert.ErrorContains(t, err, \"cannot be empty\")\n}\n\nfunc TestProcessUser_DatabaseError(t *testing.T) {\n    mockDB.EXPECT().GetUser(\"123\").Return(nil, errors.New(\"db connection failed\"))\n    \n    _, err := ProcessUser(\"123\")\n    assert.ErrorContains(t, err, \"failed to get user\")\n}\n```\n\n## :triangular_ruler: Design Principles\n\n### 1. Simple is Better Than Clever\n```go\n// YES - Clear and obvious\nif user.NeedsMigration() {\n    return migrate(user)\n}\n\n// NO - Clever but unclear\nreturn user.NeedsMigration() && migrate(user) || user\n```\n\n### 2. Explicit is Better Than Implicit\n- Clear function names\n- Obvious parameter types\n- No hidden side effects\n\n### 3. Conditions Must Be as Specific as the Intent\n- Guard clauses should match *exactly* the intended case, not just currently-known cases\n- A check like `len(tables) == 0` when you mean \"is virtual dual\" will silently fire for future zero-table cases\n- A nil return from a helper (e.g., `operatorKeyspace()` returning nil for composite operators) should not be treated as \"safe to skip\" — handle it explicitly\n- When catching MySQL error codes, match the specific codes that apply to your call site, not a broad class\n\n### 4. Zero-Value / Default Behavior Safety\n- New struct fields must not change behavior for existing callers who omit them\n- Prefer negative-polarity booleans (`PreventCrossKeyspaceReads` not `AllowCrossKeyspaceReads`) so the zero-value preserves existing behavior\n- When a flag value of `0` or empty previously meant \"disabled,\" don't change it to mean \"unlimited\" — preserve the existing semantic or make the change explicit\n- Validate mutually exclusive flags in `PreRunE` and add unit tests for invalid combinations\n\n### 5. Performance with Clarity\n- Optimize hot paths, but keep code readable\n- Preallocate slices and maps when the size is known: `make([]T, 0, len(source))`\n- Avoid duplicate work — cache results of expensive calls like `reflect.Value.MapKeys()` instead of calling twice\n- Document why, not what\n\n### 6. Fail Fast and Clearly\n- Validate inputs early\n- Return clear error messages\n- Help future debugging\n\n### 7. Interfaces Define What You Need, Not What You Provide\n- When you need something from another component, define the interface in your package\n- Don't look at what someone else provides - define exactly what you require\n- This keeps interfaces small, focused, and prevents unnecessary coupling\n- Types and their methods live together. At the top of files, use a single ```type ()``` with all type declarations inside.\n\n### 8. Go-Specific Best Practices\n- **Receiver naming** - Use consistent, short receiver names (e.g., `u *User`, not `user *User`)\n- **Package naming** - Short, descriptive, lowercase without underscores\n- **Interface naming** - Single-method interfaces end in `-er` (Reader, Writer, Handler)\n- **Context first** - Always pass `context.Context` as the first parameter\n- **Context cancellation** - Prefer `context.WithoutCancel(ctx)` over `context.Background()` when you need a non-cancellable context but still want to preserve context values (tracing, caller ID)\n- **Channels for coordination** - Use channels to coordinate goroutines, not shared memory\n- **No naked returns in non-trivial functions** - For functions with named return values, avoid bare `return` and explicitly return all result values (very small helpers are the only exception). This does not prohibit plain `return` in `func f() { ... }` when used for early-exit/guard clauses.\n- **Reduce nesting** - Prefer early returns and guard clauses over deeply nested `if` conditions\n- **Copyright header** - New Go files must include the project copyright header with the current year\n- **Always run `scripts/fmt <changed-go-files>`** before committing - this is mandatory\n- **Use format verbs precisely** - Use `%s` for strings and `%d` for integers, not `%v` for everything\n- **Structured logging** - New log messages should use structured logging with `slog`-style fields (e.g., `log.Warn(\"message\", slog.Any(\"error\", err))`) rather than printf-style logging with format strings\n- **Reuse existing helpers** - Before writing new parsing/validation code, check for existing utilities (e.g., `sqlerror` package for MySQL error codes, `mysqlctl.ParseVersionString()`, `strings.Split()`, `topoproto.TabletAliasString()` for formatting tablet aliases)\n\n## :building_construction: Vitess-Specific Conventions\n\n### Generated Code\n- **Never** directly edit files with a `Code generated ... DO NOT EDIT` header - these are generated and will be overwritten\n- Run `make codegen` to regenerate after modifying source definitions\n\n#### Protobufs\n- **Never** directly edit files under `go/vt/proto/` - they are generated from `proto/*.proto` protobuf definitions\n- After modifying `proto/*.proto` files, run `make proto` to regenerate\n- Avoid storing timestamps or time durations as integers; use `vttime.Time` for timestamps and `vttime.Duration` (or `google.protobuf.Duration`, as appropriate) for durations instead\n- Avoid storing tablet aliases as a string: use `topodata.TabletAlias`\n\n#### SQL Parser\n- **Never** directly edit these generated files in `go/vt/sqlparser/`: `sql.go`, `ast_clone.go`, `ast_copy_on_rewrite.go`, `ast_equals.go`, `ast_format_fast.go`, `ast_path.go`, `ast_rewrite.go`, `ast_visit.go`, `cached_size.go`\n- After modifying source files (e.g., `sql.y`, AST definitions), run `make codegen` to regenerate\n- Field order in AST structs matters — generated walkers visit fields in declaration order, so reordering fields changes semantic-analysis walk order and can break scope setup\n\n### Command-Line Flags\n- New flags must **not** use underscores (use hyphens instead)\n- When flags are added or modified, update the corresponding `go/flags/endtoend/` files - column/whitespace alignment matters\n\n### TabletAlias Formatting\n- Format `*topodatapb.TabletAlias` using `topoproto.TabletAliasString(alias)` in logs and error messages so that tablet aliases are human-readable\n\n### MySQL Flavor Isolation\n- MySQL-version-specific behavior belongs in the corresponding flavor implementation (e.g., MariaDB handling in the MariaDB flavor file), not in generic code\n- Be aware that MariaDB and older MySQL versions may not support all system variables (e.g., `super_read_only`) — other Vitess call sites already warn-and-continue for `ERUnknownSystemVariable`\n\n### User-Visible Changes\n- Any user-visible behavioral change — even a correctness fix — needs explicit callout in release/deployment notes\n- Removing or renaming a public API function (e.g., in `sqlparser`) is a breaking change for downstream users — call it out explicitly or keep a thin compatibility wrapper\n- Changelog summaries are for key changes all users should know about — internal implementation details don't belong there\n- Keep PRs clean of unrelated diffs (e.g., stray `package-lock.json` changes, `go.sum` without `go mod tidy`)\n\n### Release Cycle & Compatibility\nVitess ships a major release roughly every 6 months, each supported for 12 months (see the [release cycle](https://vitess.io/docs/releases/release-cycle/) docs and `doc/internal/release/versioning.md`). Some changes therefore take 1-3 release cycles to complete — recognize when a task needs multi-release staging and propose the staged plan instead of doing it all in one release.\n\n- **Backwards AND forwards compatibility** must hold between consecutive major versions ([VEP-1](https://github.com/vitessio/enhancements/blob/main/veps/vep-1.md)) — clusters run mixed-version components during rolling upgrades, and downgrade by one major version must work too. CI enforces this via the `upgrade_downgrade_test_*` workflows (the `_next_release` variants test against the next major)\n- **Deprecation is a multi-release cycle** (minimum; maintainers may extend it):\n  - Behavior changes: release N announces + warns (default unchanged, opt-in flag), release N+1 may flip the default (old behavior restorable via the now-deprecated flag), release N+2 removes the flag/old behavior\n  - Simple removals (obsolete utilities, flags): warn in release N, remove in release N+1\n  - Never remove or change a default in the same release that introduces the deprecation warning\n- **Protobuf changes must be wire-compatible in both directions**: never renumber, retype, or reuse removed field numbers; new fields must tolerate being absent (an older peer never sends them) and their zero value must preserve existing behavior. The VTGate RPC protos are public API per `doc/internal/release/versioning.md`\n- **Data written by a live system** (topology data, Vitess-internal tables, on-disk formats) is covered by the same promise — a change that breaks the upgrade *or downgrade* path of a running cluster is a breaking change even if the data is \"internal\"\n- Experimental features are excluded from the compatibility promise and the deprecation rules\n\n## :mag: Debugging & Troubleshooting\n\nWhen things don't work as expected, we debug systematically:\n\n### Debugging Strategy\n1. **Reproduce reliably** - Create a minimal failing case\n2. **Isolate the problem** - Binary search through the system\n3. **Understand the data flow** - Trace inputs and outputs\n4. **Question assumptions** - What did we assume was working?\n5. **Fix the root cause** - Not just the symptoms\n\n### Debugging Tools & Techniques\n```go\n// Use structured logging (slog-style) for new code\nlog.Info(\"Starting payment processing\",\n    slog.String(\"user_id\", userID),\n    slog.String(\"action\", \"process_payment\"),\n    slog.Float64(\"amount\", amount),\n)\n\n// Add strategic debug points\nfunc processPayment(amount float64) error {\n    if amount <= 0 {\n        return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, \"invalid amount: %f\", amount)\n    }\n\n    // More processing...\n    log.Info(\"Payment validation passed\")\n    return nil\n}\n```\n\n### When Stuck\n- Write a test that reproduces the issue\n- Add logging to understand data flow  \n- Use the debugger to step through code\n- Rubber duck explain the problem\n- Take a break and come back fresh\n\n## :recycle: Refactoring Legacy Code\n\nWhen improving existing code, we move carefully and systematically:\n\n### Refactoring Strategy\n1. **Understand first** - Read and comprehend the existing code\n2. **Add tests** - Create safety nets before changing anything  \n3. **Small steps** - Make tiny, verifiable improvements\n4. **Preserve behavior** - Keep the same external interface\n5. **Measure improvement** - Verify it's actually better\n\n### Safe Refactoring Process\n```go\n// Step 1: Add characterization tests\nfunc TestLegacyProcessor_ExistingBehavior(t *testing.T) {\n    processor := &LegacyProcessor{}\n    \n    // Document current behavior, even if it seems wrong\n    result := processor.Process(\"input\")\n    assert.Equal(t, \"weird_legacy_output\", result)\n}\n\n// Step 2: Refactor with tests passing\nfunc (p *LegacyProcessor) Process(input string) string {\n    // Improved implementation that maintains the same behavior\n    return processWithNewLogic(input)\n}\n\n// Step 3: Now we can safely change the behavior\nfunc TestProcessor_ImprovedBehavior(t *testing.T) {\n    processor := &Processor{}\n    \n    result := processor.Process(\"input\")\n    assert.Equal(t, \"expected_output\", result)\n}\n```\n\n## :arrows_counterclockwise: Development Workflow\n\n### Starting a Feature\n1. **Discuss** - \"I'm thinking about implementing X. Here's my approach...\"\n2. **Design** - Sketch out the API and key components\n3. **Test** - Write tests that define the behavior\n4. **Implement** - Make the tests pass\n5. **Review** - \"Does this make sense? Any concerns?\"\n\n### Making Changes\n1. **Small PRs** - Easier to review and less risky\n2. **Incremental** - Build features piece by piece\n3. **Always tested** - No exceptions\n4. **Clear commits** - Each commit should have a clear purpose\n\n### Git and PR Workflow\n\n**CRITICAL: Git commands are ONLY for reading state - NEVER for modifying it.**\n- **NEVER** use git commands that modify the filesystem unless explicitly told to commit\n- You may read git state: `git status`, `git log`, `git diff`, `git branch --show-current`\n- You may NOT: `git commit`, `git add`, `git reset`, `git checkout`, `git restore`, `git rebase`, `git push`, etc.\n- **ONLY commit when explicitly asked to commit**\n- Always sign git commits with the `git commit --signoff` flag\n- When asked to commit, do it once and stop\n- Only I can modify git state unless you've been given explicit permission to commit\n\n**Once a PR is created, NEVER amend commits or rewrite history.**\n- Always create new commits after PR is created\n- No `git commit --amend` after pushing to a PR branch\n- No `git rebase` that rewrites commits in the PR\n- No force pushes to PR branches\n- This keeps the PR history clean and reviewable\n\n**When asked to write a PR description:**\n1. **Use `gh` CLI** - Always use `gh pr edit <number>` to update PRs\n2. **Update both body and title** - Use `--body` and `--title` flags\n3. **Be informal, humble, and short** - Keep it conversational and to the point\n4. **Credit appropriately** - If Claude Code wrote most of it, mention that\n5. **Example format**:\n   ```\n   ## What's this?\n   [Brief explanation of the feature/fix]\n\n   ## How it works\n   [Key implementation details]\n\n   ## Usage\n   [Code examples if relevant]\n\n   ---\n   _Most of this was written by Claude Code - I just provided direction._\n   ```\n\n## :memo: Code Review Mindset\n\nWhen reviewing code (yours or mine), ask:\n- Is this the simplest solution?\n- Will this make sense in 6 months?\n- Are edge cases handled?\n- Is it well tested?\n- Does it improve the codebase?\n\n## :dart: Common Patterns\n\n### Feature Implementation\n```\nYou: \"Let's add feature X\"\nMe: \"Sounds good! What's the API going to look like? What are the main use cases?\"\n[Discussion of design]\nMe: \"Let me write some tests to clarify the behavior we want\"\n[TDD implementation]\nMe: \"Here's what I've got. What do you think?\"\n```\n\n### Bug Fixing\n```\nYou: \"We have a bug where X happens\"\nMe: \"Let's write a test that reproduces it first\"\n[Test that fails]\nMe: \"Great, now we know exactly what we're fixing\"\n[Fix implementation]\n```\n\n### Performance Work\n```\nYou: \"This seems slow\"\nMe: \"Let's benchmark it first to get a baseline\"\n[Benchmark results]\nMe: \"Now let's optimize without breaking functionality\"\n[Optimization with tests passing]\n```\n\n## :rocket: Shipping Quality\n\nBefore considering any work \"done\":\n- [ ] Tests pass and cover the feature\n- [ ] Code is clean and readable\n    - [ ] Golang code passes `scripts/fmt <changed-go-files>`\n- [ ] Edge cases are handled\n- [ ] Performance is acceptable\n- [ ] Documentation is updated if needed\n- [ ] We're both happy with it\n\nRemember: We're crafting software, not just making it work. Every line of code is an opportunity to make the system better.\n",".github/copilot-instructions.md":"# Code Review Instructions\n\n## Priority\nOnly comment on issues that affect correctness, security, or performance.\nDo NOT comment on: style preferences, minor naming conventions, formatting, or\nissues already enforced by our linter/CI pipeline.\n\n## Confidence threshold\nOnly leave a comment when you have HIGH CONFIDENCE (>80%) that a real problem exists.\nDo not flag potential issues speculatively.\n\n## Severity filter\nSkip LOW severity issues entirely. Focus on HIGH and CRITICAL issues only.\n\n## CI context\nDo not flag issues that are caught by our automated CI/CD pipeline (linting, tests, type checks).\n"},"items":[{"name":"CLAUDE.md","path":"CLAUDE.md","title":"CLAUDE.md","content":"## :handshake: Our Partnership\n\n**We're building this together.** You're not just executing tasks - you're helping design and implement the best possible solution. This means:\n\n- Challenge my suggestions when something feels wrong\n- Ask me to explain my reasoning\n- Propose alternative approaches\n- Take time to think through problems\n\n**Quality is non-negotiable.** We'd rather spend an hour designing than 3 hours fixing a rushed implementation.\n\n## :thought_balloon: Before We Code\n\nAlways discuss first:\n- What problem are we solving?\n- What's the ideal solution?\n- What tests would prove it works?\n- Are we making the codebase better?\n\n## Strict Task Adherence\n\n**Only do exactly what I ask for - nothing more, nothing less.**\n\n- Do NOT add explanatory comments unless asked\n- Do NOT make \"improvements\" or \"clean up\" code beyond the specific task\n- Do NOT add features, optimizations, or enhancements I didn't mention\n- If there is something you think should be done, suggest it, but don't do it until asked to\n\n**Red flags that indicate you're going beyond the task:**\n- \"Let me also...\"\n- \"While I'm at it...\"\n- \"I should also update...\"\n- \"Let me improve...\"\n- \"I'll also clean up...\"\n\n**If the task is complete, STOP. Don't look for more work to do.**\n\n## :test_tube: Test-Driven Development\n\nTDD isn't optional - it's how we ensure quality:\n\n### The TDD Cycle\n1. **Red** - Write a failing test that defines success\n2. **Green** - Write minimal code to pass\n3. **Refactor** - Make it clean and elegant\n\n### Example TDD Session\n```go\n// Step 1: Write the test first\nfunc TestConnectionBilateralCleanup(t *testing.T) {\n    // Define what success looks like\n    client, server := testutils.CreateConnectedTCPPair()\n    \n    // Test the behavior we want\n    client.Close()\n    \n    // Both sides should be closed\n    assert.Eventually(t, func() bool {\n        return isConnectionClosed(server)\n    })\n}\n\n// Step 2: See it fail (confirms we're testing the right thing)\n// Step 3: Implement the feature\n// Step 4: See it pass\n// Step 5: Refactor for clarity\n```\n\nTo make sure tests are easy to read, we use `github.com/stretchr/testify/assert` and `github.com/stretchr/testify/require` for assertions:\n- Use `require` (not `assert`) when the test cannot continue after a failure (e.g., `require.NoError` after setup that must succeed)\n- Use `assert.Eventually` instead of manual `time.Sleep()` and timeouts\n- Use `t.Context()` instead of `context.Background()` — it integrates with test cancellation\n- Use `t.Cleanup()` for test teardown\n- Use `assert.ErrorContains` / `require.ErrorContains` to check error messages\n- Use the `_test.go` suffix for mocks and test helpers that are only used by the current package's tests; if helpers or mocks need to be imported by other packages' tests or fuzz harnesses, put them in a normal reusable package such as `testlib` or `testutil`\n- CI timeouts must be generous (30s+) — GitHub Actions runners can be resource-starved with multi-second pauses; sub-second timeouts cause flakiness with no recourse but retry\n- Do not use t.Fatal or t.Error in tests, but instead require and assert\n\n### Test Honesty\n- A test must actually exercise the condition its name and doc claim, must fail on `main` without the fix it guards, and must not duplicate coverage that a unit test already pins down precisely. Tests that pass identically with or without the fix waste CI time and create false confidence.\n\n## :rotating_light: Error Handling Excellence\n\nError handling is not an afterthought - it's core to reliable software.\n\n### Go Error Patterns\n```go\n// YES - Clear error context with vterrors\nfunc ProcessUser(id string) (*User, error) {\n    if id == \"\" {\n        return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, \"user ID cannot be empty\")\n    }\n\n    user, err := db.GetUser(id)\n    if err != nil {\n        return nil, vterrors.Wrapf(err, \"failed to get user %s\", id)\n    }\n\n    return user, nil\n}\n\n// NO - Swallowing errors\nfunc ProcessUser(id string) *User {\n    user, _ := db.GetUser(id)  // What if this fails?\n    return user\n}\n```\n\n### Error Handling Principles\n1. **Use `vterrors`** - Prefer `vterrors` over `fmt.Errorf` or `errors` package, with an appropriate `vtrpcpb.Code` (e.g., `vtrpcpb.Code_FAILED_PRECONDITION` for unexpected input values, `vtrpcpb.Code_INTERNAL` for internal operation failures)\n2. **Wrap errors with context** - Use `vterrors.Wrapf(err, \"context\")`\n3. **Validate early** - Check inputs before doing work\n4. **Fail fast** - Don't continue with invalid state\n5. **Log appropriately** - Errors at boundaries, debug info internally\n6. **Return structured errors** - Use error types for different handling\n7. **Never silently swallow errors** - When recovering from an error (e.g., restarting replication), always log the original error before the recovery attempt so operators can trace what happened\n8. **Log with context** - Include workflow name, recovery type, tablet alias, and other identifiers in log messages — a keyspace/tablet can have many concurrent workflows\n\n### Failure-Path Safety\nMulti-step operations must not leave the system in a half-applied state:\n- If step 2 of 3 succeeds but step 3 fails, ensure step 2 is rolled back or the cleanup path handles it\n- Deferred cleanup must use bounded contexts — never `context.Background()` for operations that could hang indefinitely\n- When holding a mutex, always bound the operation with a reasonable timeout\n- Test failure paths, not just the happy path — a test that only proves \"step X ran\" without covering \"what if step X+1 fails\" is incomplete\n\n### Testing Error Paths\n```go\nfunc TestProcessUser_InvalidID(t *testing.T) {\n    _, err := ProcessUser(\"\")\n    assert.ErrorContains(t, err, \"cannot be empty\")\n}\n\nfunc TestProcessUser_DatabaseError(t *testing.T) {\n    mockDB.EXPECT().GetUser(\"123\").Return(nil, errors.New(\"db connection failed\"))\n    \n    _, err := ProcessUser(\"123\")\n    assert.ErrorContains(t, err, \"failed to get user\")\n}\n```\n\n## :triangular_ruler: Design Principles\n\n### 1. Simple is Better Than Clever\n```go\n// YES - Clear and obvious\nif user.NeedsMigration() {\n    return migrate(user)\n}\n\n// NO - Clever but unclear\nreturn user.NeedsMigration() && migrate(user) || user\n```\n\n### 2. Explicit is Better Than Implicit\n- Clear function names\n- Obvious parameter types\n- No hidden side effects\n\n### 3. Conditions Must Be as Specific as the Intent\n- Guard clauses should match *exactly* the intended case, not just currently-known cases\n- A check like `len(tables) == 0` when you mean \"is virtual dual\" will silently fire for future zero-table cases\n- A nil return from a helper (e.g., `operatorKeyspace()` returning nil for composite operators) should not be treated as \"safe to skip\" — handle it explicitly\n- When catching MySQL error codes, match the specific codes that apply to your call site, not a broad class\n\n### 4. Zero-Value / Default Behavior Safety\n- New struct fields must not change behavior for existing callers who omit them\n- Prefer negative-polarity booleans (`PreventCrossKeyspaceReads` not `AllowCrossKeyspaceReads`) so the zero-value preserves existing behavior\n- When a flag value of `0` or empty previously meant \"disabled,\" don't change it to mean \"unlimited\" — preserve the existing semantic or make the change explicit\n- Validate mutually exclusive flags in `PreRunE` and add unit tests for invalid combinations\n\n### 5. Performance with Clarity\n- Optimize hot paths, but keep code readable\n- Preallocate slices and maps when the size is known: `make([]T, 0, len(source))`\n- Avoid duplicate work — cache results of expensive calls like `reflect.Value.MapKeys()` instead of calling twice\n- Document why, not what\n\n### 6. Fail Fast and Clearly\n- Validate inputs early\n- Return clear error messages\n- Help future debugging\n\n### 7. Interfaces Define What You Need, Not What You Provide\n- When you need something from another component, define the interface in your package\n- Don't look at what someone else provides - define exactly what you require\n- This keeps interfaces small, focused, and prevents unnecessary coupling\n- Types and their methods live together. At the top of files, use a single ```type ()``` with all type declarations inside.\n\n### 8. Go-Specific Best Practices\n- **Receiver naming** - Use consistent, short receiver names (e.g., `u *User`, not `user *User`)\n- **Package naming** - Short, descriptive, lowercase without underscores\n- **Interface naming** - Single-method interfaces end in `-er` (Reader, Writer, Handler)\n- **Context first** - Always pass `context.Context` as the first parameter\n- **Context cancellation** - Prefer `context.WithoutCancel(ctx)` over `context.Background()` when you need a non-cancellable context but still want to preserve context values (tracing, caller ID)\n- **Channels for coordination** - Use channels to coordinate goroutines, not shared memory\n- **No naked returns in non-trivial functions** - For functions with named return values, avoid bare `return` and explicitly return all result values (very small helpers are the only exception). This does not prohibit plain `return` in `func f() { ... }` when used for early-exit/guard clauses.\n- **Reduce nesting** - Prefer early returns and guard clauses over deeply nested `if` conditions\n- **Copyright header** - New Go files must include the project copyright header with the current year\n- **Always run `scripts/fmt <changed-go-files>`** before committing - this is mandatory\n- **Use format verbs precisely** - Use `%s` for strings and `%d` for integers, not `%v` for everything\n- **Structured logging** - New log messages should use structured logging with `slog`-style fields (e.g., `log.Warn(\"message\", slog.Any(\"error\", err))`) rather than printf-style logging with format strings\n- **Reuse existing helpers** - Before writing new parsing/validation code, check for existing utilities (e.g., `sqlerror` package for MySQL error codes, `mysqlctl.ParseVersionString()`, `strings.Split()`, `topoproto.TabletAliasString()` for formatting tablet aliases)\n\n## :building_construction: Vitess-Specific Conventions\n\n### Generated Code\n- **Never** directly edit files with a `Code generated ... DO NOT EDIT` header - these are generated and will be overwritten\n- Run `make codegen` to regenerate after modifying source definitions\n\n#### Protobufs\n- **Never** directly edit files under `go/vt/proto/` - they are generated from `proto/*.proto` protobuf definitions\n- After modifying `proto/*.proto` files, run `make proto` to regenerate\n- Avoid storing timestamps or time durations as integers; use `vttime.Time` for timestamps and `vttime.Duration` (or `google.protobuf.Duration`, as appropriate) for durations instead\n- Avoid storing tablet aliases as a string: use `topodata.TabletAlias`\n\n#### SQL Parser\n- **Never** directly edit these generated files in `go/vt/sqlparser/`: `sql.go`, `ast_clone.go`, `ast_copy_on_rewrite.go`, `ast_equals.go`, `ast_format_fast.go`, `ast_path.go`, `ast_rewrite.go`, `ast_visit.go`, `cached_size.go`\n- After modifying source files (e.g., `sql.y`, AST definitions), run `make codegen` to regenerate\n- Field order in AST structs matters — generated walkers visit fields in declaration order, so reordering fields changes semantic-analysis walk order and can break scope setup\n\n### Command-Line Flags\n- New flags must **not** use underscores (use hyphens instead)\n- When flags are added or modified, update the corresponding `go/flags/endtoend/` files - column/whitespace alignment matters\n\n### TabletAlias Formatting\n- Format `*topodatapb.TabletAlias` using `topoproto.TabletAliasString(alias)` in logs and error messages so that tablet aliases are human-readable\n\n### MySQL Flavor Isolation\n- MySQL-version-specific behavior belongs in the corresponding flavor implementation (e.g., MariaDB handling in the MariaDB flavor file), not in generic code\n- Be aware that MariaDB and older MySQL versions may not support all system variables (e.g., `super_read_only`) — other Vitess call sites already warn-and-continue for `ERUnknownSystemVariable`\n\n### User-Visible Changes\n- Any user-visible behavioral change — even a correctness fix — needs explicit callout in release/deployment notes\n- Removing or renaming a public API function (e.g., in `sqlparser`) is a breaking change for downstream users — call it out explicitly or keep a thin compatibility wrapper\n- Changelog summaries are for key changes all users should know about — internal implementation details don't belong there\n- Keep PRs clean of unrelated diffs (e.g., stray `package-lock.json` changes, `go.sum` without `go mod tidy`)\n\n### Release Cycle & Compatibility\nVitess ships a major release roughly every 6 months, each supported for 12 months (see the [release cycle](https://vitess.io/docs/releases/release-cycle/) docs and `doc/internal/release/versioning.md`). Some changes therefore take 1-3 release cycles to complete — recognize when a task needs multi-release staging and propose the staged plan instead of doing it all in one release.\n\n- **Backwards AND forwards compatibility** must hold between consecutive major versions ([VEP-1](https://github.com/vitessio/enhancements/blob/main/veps/vep-1.md)) — clusters run mixed-version components during rolling upgrades, and downgrade by one major version must work too. CI enforces this via the `upgrade_downgrade_test_*` workflows (the `_next_release` variants test against the next major)\n- **Deprecation is a multi-release cycle** (minimum; maintainers may extend it):\n  - Behavior changes: release N announces + warns (default unchanged, opt-in flag), release N+1 may flip the default (old behavior restorable via the now-deprecated flag), release N+2 removes the flag/old behavior\n  - Simple removals (obsolete utilities, flags): warn in release N, remove in release N+1\n  - Never remove or change a default in the same release that introduces the deprecation warning\n- **Protobuf changes must be wire-compatible in both directions**: never renumber, retype, or reuse removed field numbers; new fields must tolerate being absent (an older peer never sends them) and their zero value must preserve existing behavior. The VTGate RPC protos are public API per `doc/internal/release/versioning.md`\n- **Data written by a live system** (topology data, Vitess-internal tables, on-disk formats) is covered by the same promise — a change that breaks the upgrade *or downgrade* path of a running cluster is a breaking change even if the data is \"internal\"\n- Experimental features are excluded from the compatibility promise and the deprecation rules\n\n## :mag: Debugging & Troubleshooting\n\nWhen things don't work as expected, we debug systematically:\n\n### Debugging Strategy\n1. **Reproduce reliably** - Create a minimal failing case\n2. **Isolate the problem** - Binary search through the system\n3. **Understand the data flow** - Trace inputs and outputs\n4. **Question assumptions** - What did we assume was working?\n5. **Fix the root cause** - Not just the symptoms\n\n### Debugging Tools & Techniques\n```go\n// Use structured logging (slog-style) for new code\nlog.Info(\"Starting payment processing\",\n    slog.String(\"user_id\", userID),\n    slog.String(\"action\", \"process_payment\"),\n    slog.Float64(\"amount\", amount),\n)\n\n// Add strategic debug points\nfunc processPayment(amount float64) error {\n    if amount <= 0 {\n        return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, \"invalid amount: %f\", amount)\n    }\n\n    // More processing...\n    log.Info(\"Payment validation passed\")\n    return nil\n}\n```\n\n### When Stuck\n- Write a test that reproduces the issue\n- Add logging to understand data flow  \n- Use the debugger to step through code\n- Rubber duck explain the problem\n- Take a break and come back fresh\n\n## :recycle: Refactoring Legacy Code\n\nWhen improving existing code, we move carefully and systematically:\n\n### Refactoring Strategy\n1. **Understand first** - Read and comprehend the existing code\n2. **Add tests** - Create safety nets before changing anything  \n3. **Small steps** - Make tiny, verifiable improvements\n4. **Preserve behavior** - Keep the same external interface\n5. **Measure improvement** - Verify it's actually better\n\n### Safe Refactoring Process\n```go\n// Step 1: Add characterization tests\nfunc TestLegacyProcessor_ExistingBehavior(t *testing.T) {\n    processor := &LegacyProcessor{}\n    \n    // Document current behavior, even if it seems wrong\n    result := processor.Process(\"input\")\n    assert.Equal(t, \"weird_legacy_output\", result)\n}\n\n// Step 2: Refactor with tests passing\nfunc (p *LegacyProcessor) Process(input string) string {\n    // Improved implementation that maintains the same behavior\n    return processWithNewLogic(input)\n}\n\n// Step 3: Now we can safely change the behavior\nfunc TestProcessor_ImprovedBehavior(t *testing.T) {\n    processor := &Processor{}\n    \n    result := processor.Process(\"input\")\n    assert.Equal(t, \"expected_output\", result)\n}\n```\n\n## :arrows_counterclockwise: Development Workflow\n\n### Starting a Feature\n1. **Discuss** - \"I'm thinking about implementing X. Here's my approach...\"\n2. **Design** - Sketch out the API and key components\n3. **Test** - Write tests that define the behavior\n4. **Implement** - Make the tests pass\n5. **Review** - \"Does this make sense? Any concerns?\"\n\n### Making Changes\n1. **Small PRs** - Easier to review and less risky\n2. **Incremental** - Build features piece by piece\n3. **Always tested** - No exceptions\n4. **Clear commits** - Each commit should have a clear purpose\n\n### Git and PR Workflow\n\n**CRITICAL: Git commands are ONLY for reading state - NEVER for modifying it.**\n- **NEVER** use git commands that modify the filesystem unless explicitly told to commit\n- You may read git state: `git status`, `git log`, `git diff`, `git branch --show-current`\n- You may NOT: `git commit`, `git add`, `git reset`, `git checkout`, `git restore`, `git rebase`, `git push`, etc.\n- **ONLY commit when explicitly asked to commit**\n- Always sign git commits with the `git commit --signoff` flag\n- When asked to commit, do it once and stop\n- Only I can modify git state unless you've been given explicit permission to commit\n\n**Once a PR is created, NEVER amend commits or rewrite history.**\n- Always create new commits after PR is created\n- No `git commit --amend` after pushing to a PR branch\n- No `git rebase` that rewrites commits in the PR\n- No force pushes to PR branches\n- This keeps the PR history clean and reviewable\n\n**When asked to write a PR description:**\n1. **Use `gh` CLI** - Always use `gh pr edit <number>` to update PRs\n2. **Update both body and title** - Use `--body` and `--title` flags\n3. **Be informal, humble, and short** - Keep it conversational and to the point\n4. **Credit appropriately** - If Claude Code wrote most of it, mention that\n5. **Example format**:\n   ```\n   ## What's this?\n   [Brief explanation of the feature/fix]\n\n   ## How it works\n   [Key implementation details]\n\n   ## Usage\n   [Code examples if relevant]\n\n   ---\n   _Most of this was written by Claude Code - I just provided direction._\n   ```\n\n## :memo: Code Review Mindset\n\nWhen reviewing code (yours or mine), ask:\n- Is this the simplest solution?\n- Will this make sense in 6 months?\n- Are edge cases handled?\n- Is it well tested?\n- Does it improve the codebase?\n\n## :dart: Common Patterns\n\n### Feature Implementation\n```\nYou: \"Let's add feature X\"\nMe: \"Sounds good! What's the API going to look like? What are the main use cases?\"\n[Discussion of design]\nMe: \"Let me write some tests to clarify the behavior we want\"\n[TDD implementation]\nMe: \"Here's what I've got. What do you think?\"\n```\n\n### Bug Fixing\n```\nYou: \"We have a bug where X happens\"\nMe: \"Let's write a test that reproduces it first\"\n[Test that fails]\nMe: \"Great, now we know exactly what we're fixing\"\n[Fix implementation]\n```\n\n### Performance Work\n```\nYou: \"This seems slow\"\nMe: \"Let's benchmark it first to get a baseline\"\n[Benchmark results]\nMe: \"Now let's optimize without breaking functionality\"\n[Optimization with tests passing]\n```\n\n## :rocket: Shipping Quality\n\nBefore considering any work \"done\":\n- [ ] Tests pass and cover the feature\n- [ ] Code is clean and readable\n    - [ ] Golang code passes `scripts/fmt <changed-go-files>`\n- [ ] Edge cases are handled\n- [ ] Performance is acceptable\n- [ ] Documentation is updated if needed\n- [ ] We're both happy with it\n\nRemember: We're crafting software, not just making it work. Every line of code is an opportunity to make the system better.\n","category":"root","tokens":5119},{"name":"copilot-instructions.md","path":".github/copilot-instructions.md","title":"copilot-instructions.md","content":"# Code Review Instructions\n\n## Priority\nOnly comment on issues that affect correctness, security, or performance.\nDo NOT comment on: style preferences, minor naming conventions, formatting, or\nissues already enforced by our linter/CI pipeline.\n\n## Confidence threshold\nOnly leave a comment when you have HIGH CONFIDENCE (>80%) that a real problem exists.\nDo not flag potential issues speculatively.\n\n## Severity filter\nSkip LOW severity issues entirely. Focus on HIGH and CRITICAL issues only.\n\n## CI context\nDo not flag issues that are caught by our automated CI/CD pipeline (linting, tests, type checks).\n","category":".github","tokens":152}]}