From a5efa0fbb341be8ce6017ade1393a6c26cc08ea2 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 27 Jun 2025 18:25:24 +0200 Subject: [PATCH] docs: refactor CLAUDE.md to use import system and comprehensive workflows - Restructure CLAUDE.md to use @import syntax for better organization - Create comprehensive WORKFLOWS.md with all development procedures - Add detailed documentation for OAuth2, Testing, Database, and Troubleshooting - Organize all Claude Code documentation in .claude/docs/ directory - Maintain lean main file while preserving all essential context - Include missing development server setup and migration workflows Change-Id: I641f46c0b0da384f7bbdf8b58374ecfa1572b594 Signed-off-by: Thomas Kosiewski --- .claude/docs/DATABASE.md | 258 +++++++++++++++ .claude/docs/OAUTH2.md | 159 +++++++++ .claude/docs/TESTING.md | 239 ++++++++++++++ .claude/docs/TROUBLESHOOTING.md | 227 +++++++++++++ .claude/docs/WORKFLOWS.md | 194 +++++++++++ CLAUDE.md | 561 +++++--------------------------- 6 files changed, 1157 insertions(+), 481 deletions(-) create mode 100644 .claude/docs/DATABASE.md create mode 100644 .claude/docs/OAUTH2.md create mode 100644 .claude/docs/TESTING.md create mode 100644 .claude/docs/TROUBLESHOOTING.md create mode 100644 .claude/docs/WORKFLOWS.md diff --git a/.claude/docs/DATABASE.md b/.claude/docs/DATABASE.md new file mode 100644 index 0000000000000..f6ba4bd78859b --- /dev/null +++ b/.claude/docs/DATABASE.md @@ -0,0 +1,258 @@ +# Database Development Patterns + +## Database Work Overview + +### Database Generation Process + +1. Modify SQL files in `coderd/database/queries/` +2. Run `make gen` +3. If errors about audit table, update `enterprise/audit/table.go` +4. Run `make gen` again +5. Run `make lint` to catch any remaining issues + +## Migration Guidelines + +### Creating Migration Files + +**Location**: `coderd/database/migrations/` +**Format**: `{number}_{description}.{up|down}.sql` + +- Number must be unique and sequential +- Always include both up and down migrations + +### Helper Scripts + +| Script | Purpose | +|--------|---------| +| `./coderd/database/migrations/create_migration.sh "migration name"` | Creates new migration files | +| `./coderd/database/migrations/fix_migration_numbers.sh` | Renumbers migrations to avoid conflicts | +| `./coderd/database/migrations/create_fixture.sh "fixture name"` | Creates test fixtures for migrations | + +### Database Query Organization + +- **MUST DO**: Any changes to database - adding queries, modifying queries should be done in the `coderd/database/queries/*.sql` files +- **MUST DO**: Queries are grouped in files relating to context - e.g. `prebuilds.sql`, `users.sql`, `oauth2.sql` +- After making changes to any `coderd/database/queries/*.sql` files you must run `make gen` to generate respective ORM changes + +## Handling Nullable Fields + +Use `sql.NullString`, `sql.NullBool`, etc. for optional database fields: + +```go +CodeChallenge: sql.NullString{ + String: params.codeChallenge, + Valid: params.codeChallenge != "", +} +``` + +Set `.Valid = true` when providing values. + +## Audit Table Updates + +If adding fields to auditable types: + +1. Update `enterprise/audit/table.go` +2. Add each new field with appropriate action: + - `ActionTrack`: Field should be tracked in audit logs + - `ActionIgnore`: Field should be ignored in audit logs + - `ActionSecret`: Field contains sensitive data +3. Run `make gen` to verify no audit errors + +## In-Memory Database (dbmem) Updates + +### Critical Requirements + +When adding new fields to database structs: + +- **CRITICAL**: Update `coderd/database/dbmem/dbmem.go` in-memory implementations +- The `Insert*` functions must include ALL new fields, not just basic ones +- Common issue: Tests pass with real database but fail with in-memory database due to missing field mappings +- Always verify in-memory database functions match the real database schema after migrations + +### Example Pattern + +```go +// In dbmem.go - ensure ALL fields are included +code := database.OAuth2ProviderAppCode{ + ID: arg.ID, + CreatedAt: arg.CreatedAt, + // ... existing fields ... + ResourceUri: arg.ResourceUri, // New field + CodeChallenge: arg.CodeChallenge, // New field + CodeChallengeMethod: arg.CodeChallengeMethod, // New field +} +``` + +## Database Architecture + +### Core Components + +- **PostgreSQL 13+** recommended for production +- **Migrations** managed with `migrate` +- **Database authorization** through `dbauthz` package + +### Authorization Patterns + +```go +// Public endpoints needing system access (OAuth2 registration) +app, err := api.Database.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + +// Authenticated endpoints with user context +app, err := api.Database.GetOAuth2ProviderAppByClientID(ctx, clientID) + +// System operations in middleware +roles, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), userID) +``` + +## Common Database Issues + +### Migration Issues + +1. **Migration conflicts**: Use `fix_migration_numbers.sh` to renumber +2. **Missing down migration**: Always create both up and down files +3. **Schema inconsistencies**: Verify against existing schema + +### Field Handling Issues + +1. **Nullable field errors**: Use `sql.Null*` types consistently +2. **Missing audit entries**: Update `enterprise/audit/table.go` +3. **dbmem inconsistencies**: Ensure in-memory implementations match schema + +### Query Issues + +1. **Query organization**: Group related queries in appropriate files +2. **Generated code errors**: Run `make gen` after query changes +3. **Performance issues**: Add appropriate indexes in migrations + +## Database Testing + +### Test Database Setup + +```go +func TestDatabaseFunction(t *testing.T) { + db := dbtestutil.NewDB(t) + + // Test with real database + result, err := db.GetSomething(ctx, param) + require.NoError(t, err) + require.Equal(t, expected, result) +} +``` + +### In-Memory Testing + +```go +func TestInMemoryDatabase(t *testing.T) { + db := dbmem.New() + + // Test with in-memory database + result, err := db.GetSomething(ctx, param) + require.NoError(t, err) + require.Equal(t, expected, result) +} +``` + +## Best Practices + +### Schema Design + +1. **Use appropriate data types**: VARCHAR for strings, TIMESTAMP for times +2. **Add constraints**: NOT NULL, UNIQUE, FOREIGN KEY as appropriate +3. **Create indexes**: For frequently queried columns +4. **Consider performance**: Normalize appropriately but avoid over-normalization + +### Query Writing + +1. **Use parameterized queries**: Prevent SQL injection +2. **Handle errors appropriately**: Check for specific error types +3. **Use transactions**: For related operations that must succeed together +4. **Optimize queries**: Use EXPLAIN to understand query performance + +### Migration Writing + +1. **Make migrations reversible**: Always include down migration +2. **Test migrations**: On copy of production data if possible +3. **Keep migrations small**: One logical change per migration +4. **Document complex changes**: Add comments explaining rationale + +## Advanced Patterns + +### Complex Queries + +```sql +-- Example: Complex join with aggregation +SELECT + u.id, + u.username, + COUNT(w.id) as workspace_count +FROM users u +LEFT JOIN workspaces w ON u.id = w.owner_id +WHERE u.created_at > $1 +GROUP BY u.id, u.username +ORDER BY workspace_count DESC; +``` + +### Conditional Queries + +```sql +-- Example: Dynamic filtering +SELECT * FROM oauth2_provider_apps +WHERE + ($1::text IS NULL OR name ILIKE '%' || $1 || '%') + AND ($2::uuid IS NULL OR organization_id = $2) +ORDER BY created_at DESC; +``` + +### Audit Patterns + +```go +// Example: Auditable database operation +func (q *sqlQuerier) UpdateUser(ctx context.Context, arg UpdateUserParams) (User, error) { + // Implementation here + + // Audit the change + if auditor := audit.FromContext(ctx); auditor != nil { + auditor.Record(audit.UserUpdate{ + UserID: arg.ID, + Old: oldUser, + New: newUser, + }) + } + + return newUser, nil +} +``` + +## Debugging Database Issues + +### Common Debug Commands + +```bash +# Check database connection +make test-postgres + +# Run specific database tests +go test ./coderd/database/... -run TestSpecificFunction + +# Check query generation +make gen + +# Verify audit table +make lint +``` + +### Debug Techniques + +1. **Enable query logging**: Set appropriate log levels +2. **Use database tools**: pgAdmin, psql for direct inspection +3. **Check constraints**: UNIQUE, FOREIGN KEY violations +4. **Analyze performance**: Use EXPLAIN ANALYZE for slow queries + +### Troubleshooting Checklist + +- [ ] Migration files exist (both up and down) +- [ ] `make gen` run after query changes +- [ ] Audit table updated for new fields +- [ ] In-memory database implementations updated +- [ ] Nullable fields use `sql.Null*` types +- [ ] Authorization context appropriate for endpoint type diff --git a/.claude/docs/OAUTH2.md b/.claude/docs/OAUTH2.md new file mode 100644 index 0000000000000..64374bae5457e --- /dev/null +++ b/.claude/docs/OAUTH2.md @@ -0,0 +1,159 @@ +# OAuth2 Development Guide + +## RFC Compliance Development + +### Implementing Standard Protocols + +When implementing standard protocols (OAuth2, OpenID Connect, etc.): + +1. **Fetch and Analyze Official RFCs**: + - Always read the actual RFC specifications before implementation + - Use WebFetch tool to get current RFC content for compliance verification + - Document RFC requirements in code comments + +2. **Default Values Matter**: + - Pay close attention to RFC-specified default values + - Example: RFC 7591 specifies `client_secret_basic` as default, not `client_secret_post` + - Ensure consistency between database migrations and application code + +3. **Security Requirements**: + - Follow RFC security considerations precisely + - Example: RFC 7592 prohibits returning registration access tokens in GET responses + - Implement proper error responses per protocol specifications + +4. **Validation Compliance**: + - Implement comprehensive validation per RFC requirements + - Support protocol-specific features (e.g., custom schemes for native OAuth2 apps) + - Test edge cases defined in specifications + +## OAuth2 Provider Implementation + +### OAuth2 Spec Compliance + +1. **Follow RFC 6749 for token responses** + - Use `expires_in` (seconds) not `expiry` (timestamp) in token responses + - Return proper OAuth2 error format: `{"error": "code", "error_description": "details"}` + +2. **Error Response Format** + - Create OAuth2-compliant error responses for token endpoint + - Use standard error codes: `invalid_client`, `invalid_grant`, `invalid_request` + - Avoid generic error responses for OAuth2 endpoints + +### PKCE Implementation + +- Support both with and without PKCE for backward compatibility +- Use S256 method for code challenge +- Properly validate code_verifier against stored code_challenge + +### UI Authorization Flow + +- Use POST requests for consent, not GET with links +- Avoid dependency on referer headers for security decisions +- Support proper state parameter validation + +### RFC 8707 Resource Indicators + +- Store resource parameters in database for server-side validation (opaque tokens) +- Validate resource consistency between authorization and token requests +- Support audience validation in refresh token flows +- Resource parameter is optional but must be consistent when provided + +## OAuth2 Error Handling Pattern + +```go +// Define specific OAuth2 errors +var ( + errInvalidPKCE = xerrors.New("invalid code_verifier") +) + +// Use OAuth2-compliant error responses +type OAuth2Error struct { + Error string `json:"error"` + ErrorDescription string `json:"error_description,omitempty"` +} + +// Return proper OAuth2 errors +if errors.Is(err, errInvalidPKCE) { + writeOAuth2Error(ctx, rw, http.StatusBadRequest, "invalid_grant", "The PKCE code verifier is invalid") + return +} +``` + +## Testing OAuth2 Features + +### Test Scripts + +Located in `./scripts/oauth2/`: + +- `test-mcp-oauth2.sh` - Full automated test suite +- `setup-test-app.sh` - Create test OAuth2 app +- `cleanup-test-app.sh` - Remove test app +- `generate-pkce.sh` - Generate PKCE parameters +- `test-manual-flow.sh` - Manual browser testing + +Always run the full test suite after OAuth2 changes: + +```bash +./scripts/oauth2/test-mcp-oauth2.sh +``` + +### RFC Protocol Testing + +1. **Compliance Test Coverage**: + - Test all RFC-defined error codes and responses + - Validate proper HTTP status codes for different scenarios + - Test protocol-specific edge cases (URI formats, token formats, etc.) + +2. **Security Boundary Testing**: + - Test client isolation and privilege separation + - Verify information disclosure protections + - Test token security and proper invalidation + +## Common OAuth2 Issues + +1. **OAuth2 endpoints returning wrong error format** - Ensure OAuth2 endpoints return RFC 6749 compliant errors +2. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go` +3. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly +4. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields +5. **RFC compliance failures** - Verify against actual RFC specifications, not assumptions +6. **Authorization context errors in public endpoints** - Use `dbauthz.AsSystemRestricted(ctx)` pattern +7. **Default value mismatches** - Ensure database migrations match application code defaults +8. **Bearer token authentication issues** - Check token extraction precedence and format validation +9. **URI validation failures** - Support both standard schemes and custom schemes per protocol requirements + +## Authorization Context Patterns + +```go +// Public endpoints needing system access (OAuth2 registration) +app, err := api.Database.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + +// Authenticated endpoints with user context +app, err := api.Database.GetOAuth2ProviderAppByClientID(ctx, clientID) + +// System operations in middleware +roles, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), userID) +``` + +## OAuth2/Authentication Work Patterns + +- Types go in `codersdk/oauth2.go` or similar +- Handlers go in `coderd/oauth2.go` or `coderd/identityprovider/` +- Database fields need migration + audit table updates +- Always support backward compatibility + +## Protocol Implementation Checklist + +Before completing OAuth2 or authentication feature work: + +- [ ] Verify RFC compliance by reading actual specifications +- [ ] Implement proper error response formats per protocol +- [ ] Add comprehensive validation for all protocol fields +- [ ] Test security boundaries and token handling +- [ ] Update RBAC permissions for new resources +- [ ] Add audit logging support if applicable +- [ ] Create database migrations with proper defaults +- [ ] Update in-memory database implementations +- [ ] Add comprehensive test coverage including edge cases +- [ ] Verify linting and formatting compliance +- [ ] Test both positive and negative scenarios +- [ ] Document protocol-specific patterns and requirements diff --git a/.claude/docs/TESTING.md b/.claude/docs/TESTING.md new file mode 100644 index 0000000000000..b8f92f531bb1c --- /dev/null +++ b/.claude/docs/TESTING.md @@ -0,0 +1,239 @@ +# Testing Patterns and Best Practices + +## Testing Best Practices + +### Avoiding Race Conditions + +1. **Unique Test Identifiers**: + - Never use hardcoded names in concurrent tests + - Use `time.Now().UnixNano()` or similar for unique identifiers + - Example: `fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())` + +2. **Database Constraint Awareness**: + - Understand unique constraints that can cause test conflicts + - Generate unique values for all constrained fields + - Test name isolation prevents cross-test interference + +### Testing Patterns + +- Use table-driven tests for comprehensive coverage +- Mock external dependencies +- Test both positive and negative cases +- Use `testutil.WaitLong` for timeouts in tests + +### Test Package Naming + +- **Test packages**: Use `package_test` naming (e.g., `identityprovider_test`) for black-box testing + +## RFC Protocol Testing + +### Compliance Test Coverage + +1. **Test all RFC-defined error codes and responses** +2. **Validate proper HTTP status codes for different scenarios** +3. **Test protocol-specific edge cases** (URI formats, token formats, etc.) + +### Security Boundary Testing + +1. **Test client isolation and privilege separation** +2. **Verify information disclosure protections** +3. **Test token security and proper invalidation** + +## Database Testing + +### In-Memory Database Testing + +When adding new database fields: + +- **CRITICAL**: Update `coderd/database/dbmem/dbmem.go` in-memory implementations +- The `Insert*` functions must include ALL new fields, not just basic ones +- Common issue: Tests pass with real database but fail with in-memory database due to missing field mappings +- Always verify in-memory database functions match the real database schema after migrations + +Example pattern: + +```go +// In dbmem.go - ensure ALL fields are included +code := database.OAuth2ProviderAppCode{ + ID: arg.ID, + CreatedAt: arg.CreatedAt, + // ... existing fields ... + ResourceUri: arg.ResourceUri, // New field + CodeChallenge: arg.CodeChallenge, // New field + CodeChallengeMethod: arg.CodeChallengeMethod, // New field +} +``` + +## Test Organization + +### Test File Structure + +``` +coderd/ +├── oauth2.go # Implementation +├── oauth2_test.go # Main tests +├── oauth2_test_helpers.go # Test utilities +└── oauth2_validation.go # Validation logic +``` + +### Test Categories + +1. **Unit Tests**: Test individual functions in isolation +2. **Integration Tests**: Test API endpoints with database +3. **End-to-End Tests**: Full workflow testing +4. **Race Tests**: Concurrent access testing + +## Test Commands + +### Running Tests + +| Command | Purpose | +|---------|---------| +| `make test` | Run all Go tests | +| `make test RUN=TestFunctionName` | Run specific test | +| `go test -v ./path/to/package -run TestFunctionName` | Run test with verbose output | +| `make test-postgres` | Run tests with Postgres database | +| `make test-race` | Run tests with Go race detector | +| `make test-e2e` | Run end-to-end tests | + +### Frontend Testing + +| Command | Purpose | +|---------|---------| +| `pnpm test` | Run frontend tests | +| `pnpm check` | Run code checks | + +## Common Testing Issues + +### Database-Related + +1. **Tests passing locally but failing in CI** - Check if `dbmem` implementation needs updating +2. **SQL type errors** - Use `sql.Null*` types for nullable fields +3. **Race conditions in tests** - Use unique identifiers instead of hardcoded names + +### OAuth2 Testing + +1. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go` +2. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields +3. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly + +### General Issues + +1. **Missing newlines** - Ensure files end with newline character +2. **Package naming errors** - Use `package_test` naming for test files +3. **Log message formatting errors** - Use lowercase, descriptive messages without special characters + +## Systematic Testing Approach + +### Multi-Issue Problem Solving + +When facing multiple failing tests or complex integration issues: + +1. **Identify Root Causes**: + - Run failing tests individually to isolate issues + - Use LSP tools to trace through call chains + - Check both compilation and runtime errors + +2. **Fix in Logical Order**: + - Address compilation issues first (imports, syntax) + - Fix authorization and RBAC issues next + - Resolve business logic and validation issues + - Handle edge cases and race conditions last + +3. **Verification Strategy**: + - Test each fix individually before moving to next issue + - Use `make lint` and `make gen` after database changes + - Verify RFC compliance with actual specifications + - Run comprehensive test suites before considering complete + +## Test Data Management + +### Unique Test Data + +```go +// Good: Unique identifiers prevent conflicts +clientName := fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano()) + +// Bad: Hardcoded names cause race conditions +clientName := "test-client" +``` + +### Test Cleanup + +```go +func TestSomething(t *testing.T) { + // Setup + client := coderdtest.New(t, nil) + + // Test code here + + // Cleanup happens automatically via t.Cleanup() in coderdtest +} +``` + +## Test Utilities + +### Common Test Patterns + +```go +// Table-driven tests +tests := []struct { + name string + input InputType + expected OutputType + wantErr bool +}{ + { + name: "valid input", + input: validInput, + expected: expectedOutput, + wantErr: false, + }, + // ... more test cases +} + +for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := functionUnderTest(tt.input) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tt.expected, result) + }) +} +``` + +### Test Assertions + +```go +// Use testify/require for assertions +require.NoError(t, err) +require.Equal(t, expected, actual) +require.NotNil(t, result) +require.True(t, condition) +``` + +## Performance Testing + +### Load Testing + +- Use `scaletest/` directory for load testing scenarios +- Run `./scaletest/scaletest.sh` for performance testing + +### Benchmarking + +```go +func BenchmarkFunction(b *testing.B) { + for i := 0; i < b.N; i++ { + // Function call to benchmark + _ = functionUnderTest(input) + } +} +``` + +Run benchmarks with: +```bash +go test -bench=. -benchmem ./package/path +``` diff --git a/.claude/docs/TROUBLESHOOTING.md b/.claude/docs/TROUBLESHOOTING.md new file mode 100644 index 0000000000000..443508397063d --- /dev/null +++ b/.claude/docs/TROUBLESHOOTING.md @@ -0,0 +1,227 @@ +# Troubleshooting Guide + +## Common Issues + +### Database Issues + +1. **"Audit table entry missing action"** + - **Solution**: Update `enterprise/audit/table.go` + - Add each new field with appropriate action (ActionTrack, ActionIgnore, ActionSecret) + - Run `make gen` to verify no audit errors + +2. **SQL type errors** + - **Solution**: Use `sql.Null*` types for nullable fields + - Set `.Valid = true` when providing values + - Example: + + ```go + CodeChallenge: sql.NullString{ + String: params.codeChallenge, + Valid: params.codeChallenge != "", + } + ``` + +3. **Tests passing locally but failing in CI** + - **Solution**: Check if `dbmem` implementation needs updating + - Update `coderd/database/dbmem/dbmem.go` for Insert/Update methods + - Missing fields in dbmem can cause tests to fail even if main implementation is correct + +### Testing Issues + +4. **"package should be X_test"** + - **Solution**: Use `package_test` naming for test files + - Example: `identityprovider_test` for black-box testing + +5. **Race conditions in tests** + - **Solution**: Use unique identifiers instead of hardcoded names + - Example: `fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())` + - Never use hardcoded names in concurrent tests + +6. **Missing newlines** + - **Solution**: Ensure files end with newline character + - Most editors can be configured to add this automatically + +### OAuth2 Issues + +7. **OAuth2 endpoints returning wrong error format** + - **Solution**: Ensure OAuth2 endpoints return RFC 6749 compliant errors + - Use standard error codes: `invalid_client`, `invalid_grant`, `invalid_request` + - Format: `{"error": "code", "error_description": "details"}` + +8. **OAuth2 tests failing but scripts working** + - **Solution**: Check in-memory database implementations in `dbmem.go` + - Ensure all OAuth2 fields are properly copied in Insert/Update methods + +9. **Resource indicator validation failing** + - **Solution**: Ensure database stores and retrieves resource parameters correctly + - Check both authorization code storage and token exchange handling + +10. **PKCE tests failing** + - **Solution**: Verify both authorization code storage and token exchange handle PKCE fields + - Check `CodeChallenge` and `CodeChallengeMethod` field handling + +### RFC Compliance Issues + +11. **RFC compliance failures** + - **Solution**: Verify against actual RFC specifications, not assumptions + - Use WebFetch tool to get current RFC content for compliance verification + - Read the actual RFC specifications before implementation + +12. **Default value mismatches** + - **Solution**: Ensure database migrations match application code defaults + - Example: RFC 7591 specifies `client_secret_basic` as default, not `client_secret_post` + +### Authorization Issues + +13. **Authorization context errors in public endpoints** + - **Solution**: Use `dbauthz.AsSystemRestricted(ctx)` pattern + - Example: + + ```go + // Public endpoints needing system access + app, err := api.Database.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + ``` + +### Authentication Issues + +14. **Bearer token authentication issues** + - **Solution**: Check token extraction precedence and format validation + - Ensure proper RFC 6750 Bearer Token Support implementation + +15. **URI validation failures** + - **Solution**: Support both standard schemes and custom schemes per protocol requirements + - Native OAuth2 apps may use custom schemes + +### General Development Issues + +16. **Log message formatting errors** + - **Solution**: Use lowercase, descriptive messages without special characters + - Follow Go logging conventions + +## Systematic Debugging Approach + +### Multi-Issue Problem Solving + +When facing multiple failing tests or complex integration issues: + +1. **Identify Root Causes**: + - Run failing tests individually to isolate issues + - Use LSP tools to trace through call chains + - Check both compilation and runtime errors + +2. **Fix in Logical Order**: + - Address compilation issues first (imports, syntax) + - Fix authorization and RBAC issues next + - Resolve business logic and validation issues + - Handle edge cases and race conditions last + +3. **Verification Strategy**: + - Test each fix individually before moving to next issue + - Use `make lint` and `make gen` after database changes + - Verify RFC compliance with actual specifications + - Run comprehensive test suites before considering complete + +## Debug Commands + +### Useful Debug Commands + +| Command | Purpose | +|---------|---------| +| `make lint` | Run all linters | +| `make gen` | Generate mocks, database queries | +| `make fmt` | Format all code | +| `go test -v ./path/to/package -run TestName` | Run specific test with verbose output | +| `go test -race ./...` | Run tests with race detector | + +### LSP Debugging + +| Command | Purpose | +|---------|---------| +| `mcp__go-language-server__definition symbolName` | Find function definition | +| `mcp__go-language-server__references symbolName` | Find all references | +| `mcp__go-language-server__diagnostics filePath` | Check for compilation errors | + +## Common Error Messages + +### Database Errors + +**Error**: `pq: relation "oauth2_provider_app_codes" does not exist` + +- **Cause**: Missing database migration +- **Solution**: Run database migrations, check migration files + +**Error**: `audit table entry missing action for field X` + +- **Cause**: New field added without audit table update +- **Solution**: Update `enterprise/audit/table.go` + +### Go Compilation Errors + +**Error**: `package should be identityprovider_test` + +- **Cause**: Test package naming convention violation +- **Solution**: Use `package_test` naming for black-box tests + +**Error**: `cannot use X (type Y) as type Z` + +- **Cause**: Type mismatch, often with nullable fields +- **Solution**: Use appropriate `sql.Null*` types + +### OAuth2 Errors + +**Error**: `invalid_client` but client exists + +- **Cause**: Authorization context issue +- **Solution**: Use `dbauthz.AsSystemRestricted(ctx)` for public endpoints + +**Error**: PKCE validation failing + +- **Cause**: Missing PKCE fields in database operations +- **Solution**: Ensure `CodeChallenge` and `CodeChallengeMethod` are handled + +## Prevention Strategies + +### Before Making Changes + +1. **Read the relevant documentation** +2. **Check if similar patterns exist in codebase** +3. **Understand the authorization context requirements** +4. **Plan database changes carefully** + +### During Development + +1. **Run tests frequently**: `make test` +2. **Use LSP tools for navigation**: Avoid manual searching +3. **Follow RFC specifications precisely** +4. **Update audit tables when adding database fields** + +### Before Committing + +1. **Run full test suite**: `make test` +2. **Check linting**: `make lint` +3. **Verify formatting**: `make fmt` +4. **Test with race detector**: `make test-race` + +## Getting Help + +### Internal Resources + +- Check existing similar implementations in codebase +- Use LSP tools to understand code relationships +- Read related test files for expected behavior + +### External Resources + +- Official RFC specifications for protocol compliance +- Go documentation for language features +- PostgreSQL documentation for database issues + +### Debug Information Collection + +When reporting issues, include: + +1. **Exact error message** +2. **Steps to reproduce** +3. **Relevant code snippets** +4. **Test output (if applicable)** +5. **Environment information** (OS, Go version, etc.) diff --git a/.claude/docs/WORKFLOWS.md b/.claude/docs/WORKFLOWS.md new file mode 100644 index 0000000000000..2e14deb413256 --- /dev/null +++ b/.claude/docs/WORKFLOWS.md @@ -0,0 +1,194 @@ +# Development Workflows and Guidelines + +## Quick Start Checklist for New Features + +### Before Starting + +- [ ] Run `git pull` to ensure you're on latest code +- [ ] Check if feature touches database - you'll need migrations +- [ ] Check if feature touches audit logs - update `enterprise/audit/table.go` + +## Development Server + +### Starting Development Mode + +- **Use `./scripts/develop.sh` to start Coder in development mode** +- This automatically builds and runs with `--dev` flag and proper access URL +- **⚠️ Do NOT manually run `make build && ./coder server --dev` - use the script instead** + +### Development Workflow + +1. **Always start with the development script**: `./scripts/develop.sh` +2. **Make changes** to your code +3. **The script will automatically rebuild** and restart as needed +4. **Access the development server** at the URL provided by the script + +## Code Style Guidelines + +### Go Style + +- Follow [Effective Go](https://go.dev/doc/effective_go) and [Go's Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) +- Use `gofumpt` for formatting +- Create packages when used during implementation +- Validate abstractions against implementations +- **Test packages**: Use `package_test` naming (e.g., `identityprovider_test`) for black-box testing + +### Error Handling + +- Use descriptive error messages +- Wrap errors with context +- Propagate errors appropriately +- Use proper error types +- Pattern: `xerrors.Errorf("failed to X: %w", err)` + +### Naming Conventions + +- Use clear, descriptive names +- Abbreviate only when obvious +- Follow Go and TypeScript naming conventions + +### Comments + +- Document exported functions, types, and non-obvious logic +- Follow JSDoc format for TypeScript +- Use godoc format for Go code + +## Database Migration Workflows + +### Migration Guidelines + +1. **Create migration files**: + - Location: `coderd/database/migrations/` + - Format: `{number}_{description}.{up|down}.sql` + - Number must be unique and sequential + - Always include both up and down migrations + +2. **Use helper scripts**: + - `./coderd/database/migrations/create_migration.sh "migration name"` - Creates new migration files + - `./coderd/database/migrations/fix_migration_numbers.sh` - Renumbers migrations to avoid conflicts + - `./coderd/database/migrations/create_fixture.sh "fixture name"` - Creates test fixtures for migrations + +3. **Update database queries**: + - **MUST DO**: Any changes to database - adding queries, modifying queries should be done in the `coderd/database/queries/*.sql` files + - **MUST DO**: Queries are grouped in files relating to context - e.g. `prebuilds.sql`, `users.sql`, `oauth2.sql` + - After making changes to any `coderd/database/queries/*.sql` files you must run `make gen` to generate respective ORM changes + +4. **Handle nullable fields**: + - Use `sql.NullString`, `sql.NullBool`, etc. for optional database fields + - Set `.Valid = true` when providing values + +5. **Audit table updates**: + - If adding fields to auditable types, update `enterprise/audit/table.go` + - Add each new field with appropriate action (ActionTrack, ActionIgnore, ActionSecret) + - Run `make gen` to verify no audit errors + +6. **In-memory database (dbmem) updates**: + - When adding new fields to database structs, ensure `dbmem` implementation copies all fields + - Check `coderd/database/dbmem/dbmem.go` for Insert/Update methods + - Missing fields in dbmem can cause tests to fail even if main implementation is correct + +### Database Generation Process + +1. Modify SQL files in `coderd/database/queries/` +2. Run `make gen` +3. If errors about audit table, update `enterprise/audit/table.go` +4. Run `make gen` again +5. Run `make lint` to catch any remaining issues + +## API Development Workflow + +### Adding New API Endpoints + +1. **Define types** in `codersdk/` package +2. **Add handler** in appropriate `coderd/` file +3. **Register route** in `coderd/coderd.go` +4. **Add tests** in `coderd/*_test.go` files +5. **Update OpenAPI** by running `make gen` + +## Testing Workflows + +### Test Execution + +- Run full test suite: `make test` +- Run specific test: `make test RUN=TestFunctionName` +- Run with Postgres: `make test-postgres` +- Run with race detector: `make test-race` +- Run end-to-end tests: `make test-e2e` + +### Test Development + +- Use table-driven tests for comprehensive coverage +- Mock external dependencies +- Test both positive and negative cases +- Use `testutil.WaitLong` for timeouts in tests +- Always use `t.Parallel()` in tests + +## Commit Style + +- Follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/) +- Format: `type(scope): message` +- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `test`, `chore` +- Keep message titles concise (~70 characters) +- Use imperative, present tense in commit titles + +## Code Navigation and Investigation + +### Using Go LSP Tools (STRONGLY RECOMMENDED) + +**IMPORTANT**: Always use Go LSP tools for code navigation and understanding. These tools provide accurate, real-time analysis of the codebase and should be your first choice for code investigation. + +1. **Find function definitions** (USE THIS FREQUENTLY): + - `mcp__go-language-server__definition symbolName` + - Example: `mcp__go-language-server__definition getOAuth2ProviderAppAuthorize` + - Quickly jump to function implementations across packages + +2. **Find symbol references** (ESSENTIAL FOR UNDERSTANDING IMPACT): + - `mcp__go-language-server__references symbolName` + - Locate all usages of functions, types, or variables + - Critical for refactoring and understanding data flow + +3. **Get symbol information**: + - `mcp__go-language-server__hover filePath line column` + - Get type information and documentation at specific positions + +### Investigation Strategy (LSP-First Approach) + +1. **Start with route registration** in `coderd/coderd.go` to understand API endpoints +2. **Use LSP `definition` lookup** to trace from route handlers to actual implementations +3. **Use LSP `references`** to understand how functions are called throughout the codebase +4. **Follow the middleware chain** using LSP tools to understand request processing flow +5. **Check test files** for expected behavior and error patterns + +## Troubleshooting Development Issues + +### Common Issues + +1. **Development server won't start** - Use `./scripts/develop.sh` instead of manual commands +2. **Database migration errors** - Check migration file format and use helper scripts +3. **Test failures after database changes** - Update `dbmem` implementations +4. **Audit table errors** - Update `enterprise/audit/table.go` with new fields +5. **OAuth2 compliance issues** - Ensure RFC-compliant error responses + +### Debug Commands + +- Check linting: `make lint` +- Generate code: `make gen` +- Format code: `make fmt` +- Clean build: `make clean` + +## Development Environment Setup + +### Prerequisites + +- Go (version specified in go.mod) +- Node.js and pnpm for frontend development +- PostgreSQL for database testing +- Docker for containerized testing + +### First Time Setup + +1. Clone the repository +2. Run `./scripts/develop.sh` to start development server +3. Access the development URL provided +4. Create admin user as prompted +5. Begin development diff --git a/CLAUDE.md b/CLAUDE.md index 970cb4174f6ba..8f7e8d45089a2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,37 +1,25 @@ # Coder Development Guidelines -Read [cursor rules](.cursorrules). - -## Quick Start Checklist for New Features - -### Before Starting - -- [ ] Run `git pull` to ensure you're on latest code -- [ ] Check if feature touches database - you'll need migrations -- [ ] Check if feature touches audit logs - update `enterprise/audit/table.go` - -## Development Server - -### Starting Development Mode - -- Use `./scripts/develop.sh` to start Coder in development mode -- This automatically builds and runs with `--dev` flag and proper access URL -- Do NOT manually run `make build && ./coder server --dev` - use the script instead - -## Build/Test/Lint Commands - -### Main Commands - -- `make build` or `make build-fat` - Build all "fat" binaries (includes "server" functionality) -- `make build-slim` - Build "slim" binaries -- `make test` - Run Go tests -- `make test RUN=TestFunctionName` or `go test -v ./path/to/package -run TestFunctionName` - Test single -- `make test-postgres` - Run tests with Postgres database -- `make test-race` - Run tests with Go race detector -- `make test-e2e` - Run end-to-end tests -- `make lint` - Run all linters -- `make fmt` - Format all code -- `make gen` - Generates mocks, database queries and other auto-generated files +@.claude/docs/WORKFLOWS.md +@.cursorrules +@README.md +@package.json + +## 🚀 Essential Commands + +| Task | Command | Notes | +|------|---------|-------| +| **Development** | `./scripts/develop.sh` | ⚠️ Don't use manual build | +| **Build** | `make build` | Fat binaries (includes server) | +| **Build Slim** | `make build-slim` | Slim binaries | +| **Test** | `make test` | Full test suite | +| **Test Single** | `make test RUN=TestName` | Faster than full suite | +| **Test Postgres** | `make test-postgres` | Run tests with Postgres database | +| **Test Race** | `make test-race` | Run tests with Go race detector | +| **Lint** | `make lint` | Always run after changes | +| **Generate** | `make gen` | After database changes | +| **Format** | `make fmt` | Auto-format code | +| **Clean** | `make clean` | Clean build artifacts | ### Frontend Commands (site directory) @@ -42,486 +30,97 @@ Read [cursor rules](.cursorrules). - `pnpm lint` - Lint frontend code - `pnpm test` - Run frontend tests -## Code Style Guidelines +### Documentation Commands -### Go +- `pnpm run format-docs` - Format markdown tables in docs +- `pnpm run lint-docs` - Lint and fix markdown files +- `pnpm run storybook` - Run Storybook (from site directory) -- Follow [Effective Go](https://go.dev/doc/effective_go) and [Go's Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) -- Use `gofumpt` for formatting -- Create packages when used during implementation -- Validate abstractions against implementations -- **Test packages**: Use `package_test` naming (e.g., `identityprovider_test`) for black-box testing - -### Error Handling - -- Use descriptive error messages -- Wrap errors with context -- Propagate errors appropriately -- Use proper error types -- (`xerrors.Errorf("failed to X: %w", err)`) - -### Naming - -- Use clear, descriptive names -- Abbreviate only when obvious -- Follow Go and TypeScript naming conventions - -### Comments - -- Document exported functions, types, and non-obvious logic -- Follow JSDoc format for TypeScript -- Use godoc format for Go code - -## Commit Style - -- Follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/) -- Format: `type(scope): message` -- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `test`, `chore` -- Keep message titles concise (~70 characters) -- Use imperative, present tense in commit titles - -## Database Work - -### Migration Guidelines - -1. **Create migration files**: - - Location: `coderd/database/migrations/` - - Format: `{number}_{description}.{up|down}.sql` - - Number must be unique and sequential - - Always include both up and down migrations - - **Use helper scripts**: - - `./coderd/database/migrations/create_migration.sh "migration name"` - Creates new migration files - - `./coderd/database/migrations/fix_migration_numbers.sh` - Renumbers migrations to avoid conflicts - - `./coderd/database/migrations/create_fixture.sh "fixture name"` - Creates test fixtures for migrations - -2. **Update database queries**: - - MUST DO! Any changes to database - adding queries, modifying queries should be done in the `coderd/database/queries/*.sql` files - - MUST DO! Queries are grouped in files relating to context - e.g. `prebuilds.sql`, `users.sql`, `oauth2.sql` - - After making changes to any `coderd/database/queries/*.sql` files you must run `make gen` to generate respective ORM changes - -3. **Handle nullable fields**: - - Use `sql.NullString`, `sql.NullBool`, etc. for optional database fields - - Set `.Valid = true` when providing values - - Example: - - ```go - CodeChallenge: sql.NullString{ - String: params.codeChallenge, - Valid: params.codeChallenge != "", - } - ``` - -4. **Audit table updates**: - - If adding fields to auditable types, update `enterprise/audit/table.go` - - Add each new field with appropriate action (ActionTrack, ActionIgnore, ActionSecret) - - Run `make gen` to verify no audit errors - -5. **In-memory database (dbmem) updates**: - - When adding new fields to database structs, ensure `dbmem` implementation copies all fields - - Check `coderd/database/dbmem/dbmem.go` for Insert/Update methods - - Missing fields in dbmem can cause tests to fail even if main implementation is correct - -### Database Generation Process - -1. Modify SQL files in `coderd/database/queries/` +## 🔧 Critical Patterns + +### Database Changes (ALWAYS FOLLOW) + +1. Modify `coderd/database/queries/*.sql` files 2. Run `make gen` -3. If errors about audit table, update `enterprise/audit/table.go` +3. If audit errors: update `enterprise/audit/table.go` 4. Run `make gen` again -5. Run `make lint` to catch any remaining issues +5. Update `coderd/database/dbmem/dbmem.go` in-memory implementations -### In-Memory Database Testing +### LSP Navigation (USE FIRST) -When adding new database fields: +- **Find definitions**: `mcp__go-language-server__definition symbolName` +- **Find references**: `mcp__go-language-server__references symbolName` +- **Get type info**: `mcp__go-language-server__hover filePath line column` -- **CRITICAL**: Update `coderd/database/dbmem/dbmem.go` in-memory implementations -- The `Insert*` functions must include ALL new fields, not just basic ones -- Common issue: Tests pass with real database but fail with in-memory database due to missing field mappings -- Always verify in-memory database functions match the real database schema after migrations - -Example pattern: +### OAuth2 Error Handling ```go -// In dbmem.go - ensure ALL fields are included -code := database.OAuth2ProviderAppCode{ - ID: arg.ID, - CreatedAt: arg.CreatedAt, - // ... existing fields ... - ResourceUri: arg.ResourceUri, // New field - CodeChallenge: arg.CodeChallenge, // New field - CodeChallengeMethod: arg.CodeChallengeMethod, // New field -} +// OAuth2-compliant error responses +writeOAuth2Error(ctx, rw, http.StatusBadRequest, "invalid_grant", "description") ``` -## Architecture - -### Core Components - -- **coderd**: Main API service connecting workspaces, provisioners, and users -- **provisionerd**: Execution context for infrastructure-modifying providers -- **Agents**: Services in remote workspaces providing features like SSH and port forwarding -- **Workspaces**: Cloud resources defined by Terraform - -### Adding New API Endpoints - -1. **Define types** in `codersdk/` package -2. **Add handler** in appropriate `coderd/` file -3. **Register route** in `coderd/coderd.go` -4. **Add tests** in `coderd/*_test.go` files -5. **Update OpenAPI** by running `make gen` - -## Sub-modules - -### Template System - -- Templates define infrastructure for workspaces using Terraform -- Environment variables pass context between Coder and templates -- Official modules extend development environments - -### RBAC System - -- Permissions defined at site, organization, and user levels -- Object-Action model protects resources -- Built-in roles: owner, member, auditor, templateAdmin -- Permission format: `?...` - -### Database - -- PostgreSQL 13+ recommended for production -- Migrations managed with `migrate` -- Database authorization through `dbauthz` package - -## Frontend - -The frontend is contained in the site folder. - -For building Frontend refer to [this document](docs/about/contributing/frontend.md) - -## RFC Compliance Development - -### Implementing Standard Protocols - -When implementing standard protocols (OAuth2, OpenID Connect, etc.): - -1. **Fetch and Analyze Official RFCs**: - - Always read the actual RFC specifications before implementation - - Use WebFetch tool to get current RFC content for compliance verification - - Document RFC requirements in code comments - -2. **Default Values Matter**: - - Pay close attention to RFC-specified default values - - Example: RFC 7591 specifies `client_secret_basic` as default, not `client_secret_post` - - Ensure consistency between database migrations and application code - -3. **Security Requirements**: - - Follow RFC security considerations precisely - - Example: RFC 7592 prohibits returning registration access tokens in GET responses - - Implement proper error responses per protocol specifications - -4. **Validation Compliance**: - - Implement comprehensive validation per RFC requirements - - Support protocol-specific features (e.g., custom schemes for native OAuth2 apps) - - Test edge cases defined in specifications - -## Common Patterns - -### OAuth2/Authentication Work - -- Types go in `codersdk/oauth2.go` or similar -- Handlers go in `coderd/oauth2.go` or `coderd/identityprovider/` -- Database fields need migration + audit table updates -- Always support backward compatibility - -## OAuth2 Development - -### OAuth2 Provider Implementation - -When working on OAuth2 provider features: - -1. **OAuth2 Spec Compliance**: - - Follow RFC 6749 for token responses - - Use `expires_in` (seconds) not `expiry` (timestamp) in token responses - - Return proper OAuth2 error format: `{"error": "code", "error_description": "details"}` - -2. **Error Response Format**: - - Create OAuth2-compliant error responses for token endpoint - - Use standard error codes: `invalid_client`, `invalid_grant`, `invalid_request` - - Avoid generic error responses for OAuth2 endpoints - -3. **Testing OAuth2 Features**: - - Use scripts in `./scripts/oauth2/` for testing - - Run `./scripts/oauth2/test-mcp-oauth2.sh` for comprehensive tests - - Manual testing: use `./scripts/oauth2/test-manual-flow.sh` - -4. **PKCE Implementation**: - - Support both with and without PKCE for backward compatibility - - Use S256 method for code challenge - - Properly validate code_verifier against stored code_challenge - -5. **UI Authorization Flow**: - - Use POST requests for consent, not GET with links - - Avoid dependency on referer headers for security decisions - - Support proper state parameter validation - -6. **RFC 8707 Resource Indicators**: - - Store resource parameters in database for server-side validation (opaque tokens) - - Validate resource consistency between authorization and token requests - - Support audience validation in refresh token flows - - Resource parameter is optional but must be consistent when provided - -### OAuth2 Error Handling Pattern +### Authorization Context ```go -// Define specific OAuth2 errors -var ( - errInvalidPKCE = xerrors.New("invalid code_verifier") -) - -// Use OAuth2-compliant error responses -type OAuth2Error struct { - Error string `json:"error"` - ErrorDescription string `json:"error_description,omitempty"` -} - -// Return proper OAuth2 errors -if errors.Is(err, errInvalidPKCE) { - writeOAuth2Error(ctx, rw, http.StatusBadRequest, "invalid_grant", "The PKCE code verifier is invalid") - return -} -``` - -### Testing Patterns - -- Use table-driven tests for comprehensive coverage -- Mock external dependencies -- Test both positive and negative cases -- Use `testutil.WaitLong` for timeouts in tests - -## Testing Best Practices - -### Avoiding Race Conditions - -1. **Unique Test Identifiers**: - - Never use hardcoded names in concurrent tests - - Use `time.Now().UnixNano()` or similar for unique identifiers - - Example: `fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())` - -2. **Database Constraint Awareness**: - - Understand unique constraints that can cause test conflicts - - Generate unique values for all constrained fields - - Test name isolation prevents cross-test interference - -### RFC Protocol Testing - -1. **Compliance Test Coverage**: - - Test all RFC-defined error codes and responses - - Validate proper HTTP status codes for different scenarios - - Test protocol-specific edge cases (URI formats, token formats, etc.) - -2. **Security Boundary Testing**: - - Test client isolation and privilege separation - - Verify information disclosure protections - - Test token security and proper invalidation - -## Code Navigation and Investigation - -### Using Go LSP Tools (STRONGLY RECOMMENDED) - -**IMPORTANT**: Always use Go LSP tools for code navigation and understanding. These tools provide accurate, real-time analysis of the codebase and should be your first choice for code investigation. - -When working with the Coder codebase, leverage Go Language Server Protocol tools for efficient code navigation: - -1. **Find function definitions** (USE THIS FREQUENTLY): - - ```none - mcp__go-language-server__definition symbolName - ``` - - - Example: `mcp__go-language-server__definition getOAuth2ProviderAppAuthorize` - - Example: `mcp__go-language-server__definition ExtractAPIKeyMW` - - Quickly jump to function implementations across packages - - **Use this when**: You see a function call and want to understand its implementation - - **Tip**: Include package prefix if symbol is ambiguous (e.g., `httpmw.ExtractAPIKeyMW`) - -2. **Find symbol references** (ESSENTIAL FOR UNDERSTANDING IMPACT): - - ```none - mcp__go-language-server__references symbolName - ``` - - - Example: `mcp__go-language-server__references APITokenFromRequest` - - Locate all usages of functions, types, or variables - - Understand code dependencies and call patterns - - **Use this when**: Making changes to understand what code might be affected - - **Critical for**: Refactoring, deprecating functions, or understanding data flow - -3. **Get symbol information** (HELPFUL FOR TYPE INFO): - - ```none - mcp__go-language-server__hover filePath line column - ``` - - - Example: `mcp__go-language-server__hover /Users/thomask33/Projects/coder/coderd/httpmw/apikey.go 560 25` - - Get type information and documentation at specific positions - - **Use this when**: You need to understand the type of a variable or return value - -4. **Edit files using LSP** (WHEN MAKING TARGETED CHANGES): - - ```none - mcp__go-language-server__edit_file filePath edits - ``` - - - Make precise edits using line numbers - - **Use this when**: You need to make small, targeted changes to specific lines - -5. **Get diagnostics** (ALWAYS CHECK AFTER CHANGES): - - ```none - mcp__go-language-server__diagnostics filePath - ``` - - - Check for compilation errors, unused imports, etc. - - **Use this when**: After making changes to ensure code is still valid - -### LSP Tool Usage Priority - -**ALWAYS USE THESE TOOLS FIRST**: - -- **Use LSP `definition`** instead of manual searching for function implementations -- **Use LSP `references`** instead of grep when looking for function/type usage -- **Use LSP `hover`** to understand types and signatures -- **Use LSP `diagnostics`** after making changes to check for errors - -**When to use other tools**: - -- **Use Grep for**: Text-based searches, finding patterns across files, searching comments -- **Use Bash for**: Running tests, git commands, build operations -- **Use Read tool for**: Reading configuration files, documentation, non-Go files - -### Investigation Strategy (LSP-First Approach) - -1. **Start with route registration** in `coderd/coderd.go` to understand API endpoints -2. **Use LSP `definition` lookup** to trace from route handlers to actual implementations -3. **Use LSP `references`** to understand how functions are called throughout the codebase -4. **Follow the middleware chain** using LSP tools to understand request processing flow -5. **Check test files** for expected behavior and error patterns -6. **Use LSP `diagnostics`** to ensure your changes don't break compilation - -### Common LSP Workflows - -**Understanding a new feature**: - -1. Use `grep` to find the main entry point (e.g., route registration) -2. Use LSP `definition` to jump to handler implementation -3. Use LSP `references` to see how the handler is used -4. Use LSP `definition` on each function call within the handler - -**Making changes to existing code**: - -1. Use LSP `references` to understand the impact of your changes -2. Use LSP `definition` to understand the current implementation -3. Make your changes using `Edit` or LSP `edit_file` -4. Use LSP `diagnostics` to verify your changes compile correctly -5. Run tests to ensure functionality still works - -**Debugging issues**: - -1. Use LSP `definition` to find the problematic function -2. Use LSP `references` to trace how the function is called -3. Use LSP `hover` to understand parameter types and return values -4. Use `Read` to examine the full context around the issue - -## Testing Scripts - -### OAuth2 Test Scripts - -Located in `./scripts/oauth2/`: +// Public endpoints needing system access +app, err := api.Database.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) -- `test-mcp-oauth2.sh` - Full automated test suite -- `setup-test-app.sh` - Create test OAuth2 app -- `cleanup-test-app.sh` - Remove test app -- `generate-pkce.sh` - Generate PKCE parameters -- `test-manual-flow.sh` - Manual browser testing +// Authenticated endpoints with user context +app, err := api.Database.GetOAuth2ProviderAppByClientID(ctx, clientID) +``` -Always run the full test suite after OAuth2 changes: +## 📋 Quick Reference -```bash -./scripts/oauth2/test-mcp-oauth2.sh -``` +*Full workflows available in imported WORKFLOWS.md* -## Troubleshooting +### New Feature Checklist -### Common Issues +- [ ] Run `git pull` to ensure latest code +- [ ] Check if feature touches database - you'll need migrations +- [ ] Check if feature touches audit logs - update `enterprise/audit/table.go` -1. **"Audit table entry missing action"** - Update `enterprise/audit/table.go` -2. **"package should be X_test"** - Use `package_test` naming for test files -3. **SQL type errors** - Use `sql.Null*` types for nullable fields -4. **Missing newlines** - Ensure files end with newline character -5. **Tests passing locally but failing in CI** - Check if `dbmem` implementation needs updating -6. **OAuth2 endpoints returning wrong error format** - Ensure OAuth2 endpoints return RFC 6749 compliant errors -7. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go` -8. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly -9. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields -10. **Race conditions in tests** - Use unique identifiers instead of hardcoded names -11. **RFC compliance failures** - Verify against actual RFC specifications, not assumptions -12. **Authorization context errors in public endpoints** - Use `dbauthz.AsSystemRestricted(ctx)` pattern -13. **Default value mismatches** - Ensure database migrations match application code defaults -14. **Bearer token authentication issues** - Check token extraction precedence and format validation -15. **URI validation failures** - Support both standard schemes and custom schemes per protocol requirements -16. **Log message formatting errors** - Use lowercase, descriptive messages without special characters +## 🏗️ Architecture -## Systematic Debugging Approach +- **coderd**: Main API service +- **provisionerd**: Infrastructure provisioning +- **Agents**: Workspace services (SSH, port forwarding) +- **Database**: PostgreSQL with `dbauthz` authorization -### Multi-Issue Problem Solving +## 🧪 Testing -When facing multiple failing tests or complex integration issues: +### Race Condition Prevention -1. **Identify Root Causes**: - - Run failing tests individually to isolate issues - - Use LSP tools to trace through call chains - - Check both compilation and runtime errors +- Use unique identifiers: `fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())` +- Never use hardcoded names in concurrent tests -2. **Fix in Logical Order**: - - Address compilation issues first (imports, syntax) - - Fix authorization and RBAC issues next - - Resolve business logic and validation issues - - Handle edge cases and race conditions last +### OAuth2 Testing -3. **Verification Strategy**: - - Test each fix individually before moving to next issue - - Use `make lint` and `make gen` after database changes - - Verify RFC compliance with actual specifications - - Run comprehensive test suites before considering complete +- Full suite: `./scripts/oauth2/test-mcp-oauth2.sh` +- Manual testing: `./scripts/oauth2/test-manual-flow.sh` -### Authorization Context Patterns +## 🎯 Code Style -Common patterns for different endpoint types: +*Detailed guidelines in imported WORKFLOWS.md* -```go -// Public endpoints needing system access (OAuth2 registration) -app, err := api.Database.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) +- Follow [Uber Go Style Guide](https://github.com/uber-go/guide/blob/master/style.md) +- Use `gofumpt` for formatting +- Commit format: `type(scope): message` -// Authenticated endpoints with user context -app, err := api.Database.GetOAuth2ProviderAppByClientID(ctx, clientID) +## 📚 Detailed Development Guides -// System operations in middleware -roles, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), userID) -``` +@.claude/docs/OAUTH2.md +@.claude/docs/TESTING.md +@.claude/docs/TROUBLESHOOTING.md +@.claude/docs/DATABASE.md -## Protocol Implementation Checklist +## 🚨 Common Pitfalls -### OAuth2/Authentication Protocol Implementation +1. **Audit table errors** → Update `enterprise/audit/table.go` +2. **OAuth2 errors** → Return RFC-compliant format +3. **dbmem failures** → Update in-memory implementations +4. **Race conditions** → Use unique test identifiers +5. **Missing newlines** → Ensure files end with newline -Before completing OAuth2 or authentication feature work: +--- -- [ ] Verify RFC compliance by reading actual specifications -- [ ] Implement proper error response formats per protocol -- [ ] Add comprehensive validation for all protocol fields -- [ ] Test security boundaries and token handling -- [ ] Update RBAC permissions for new resources -- [ ] Add audit logging support if applicable -- [ ] Create database migrations with proper defaults -- [ ] Update in-memory database implementations -- [ ] Add comprehensive test coverage including edge cases -- [ ] Verify linting and formatting compliance -- [ ] Test both positive and negative scenarios -- [ ] Document protocol-specific patterns and requirements +*This file stays lean and actionable. Detailed workflows and explanations are imported automatically.*