Sync Gateway Code Review

Code review assistant for Couchbase Sync Gateway that enforces project-specific standards around API documentation, logging practices, data privacy, performance, and code quality.

What This Skill Does

This skill provides structured code review guidance for the Couchbase Sync Gateway project, which manages access control and data synchronization between Couchbase Lite and Couchbase Server. It ensures that pull requests adhere to critical project standards around API documentation, logging hygiene, PII protection, concurrency safety, and code maintainability.

Instructions

When reviewing code changes in the Couchbase Sync Gateway repository, systematically check the following aspects:

1. OpenAPI Specification Updates

**Check if:** Changes touch REST API handlers, query parameters, request/response structs, or endpoint behavior.

**Ensure:** The corresponding OpenAPI specifications in the `docs/api` directory are updated to reflect:

**Why:** API documentation must stay synchronized with implementation to maintain accurate external documentation and API contracts.

2. Development Logging Removal

**Check if:** Code contains temporary debugging statements using:

**Ensure:** These are either:

- `base.Infof` - informational messages

- `base.Warnf` - warning conditions

- `base.Debugf` - debug-level details

- `base.Errorf` - error conditions

**Why:** Standard library logging bypasses Sync Gateway's structured logging system, losing context, log levels, and configurability.

3. User Data Redaction

**Check if:** Log messages include any of the following User Data types:

**Ensure:** All User Data values are wrapped with `base.UD()` helper function to enable redaction.

**Example:**

```go

// Bad - exposes PII

base.Infof("Processing document %s for user %s", docID, username)

// Good - enables redaction

base.Infof("Processing document %s for user %s", base.UD(docID), base.UD(username))

```

**Why:** Privacy compliance and security best practices require the ability to redact sensitive information from logs in production environments.

4. Performance and Concurrency

**Check if:** Changes involve:

**Ensure:**

**Why:** Sync Gateway is a high-performance concurrent system. Poor concurrency practices can lead to performance degradation, data corruption, or system hangs.

5. Code Comments Quality

**Review:** All code comments for clarity and purpose.

**Ensure:** Comments explain:

**Avoid:** Comments that simply restate the code:

```go

// Bad - restates the obvious

// Increment the counter

counter++

// Good - explains reasoning

// Increment counter before lock to avoid race with reader goroutines

counter++

```

**Why:** Good comments maintain code understanding over time and help future maintainers understand design decisions.

6. Loop Safety

**Check:** All `for` loops for proper exit conditions.

**Ensure:**

**Prefer:**

```go

// Good - exit condition is clear

for i := 0; i < maxRetries && !success; i++ {

success = attemptOperation()

}

// Avoid - exit condition hidden in loop body

for {

if conditionMet {

break

}

// ...complex logic...

}

```

**Why:** Infinite loops can cause production incidents. Explicit exit conditions in the loop declaration make the termination logic immediately visible.

Usage Example

When reviewing a pull request that adds a new REST endpoint:

1. **API Documentation:** Verify `docs/api/openapi.yaml` includes the new endpoint specification

2. **Logging:** Check that any logging uses `base.*` functions, not `fmt.Printf`

3. **User Data:** Ensure document IDs in logs are wrapped with `base.UD(docID)`

4. **Concurrency:** Review any mutex usage for minimal lock duration

5. **Comments:** Verify comments explain *why* certain validation logic exists

6. **Loops:** Check retry loops have explicit maximum iteration counts

Important Notes