|
1 | 1 | # Agent Guidelines |
2 | 2 |
|
3 | 3 | ## Architecture Overview |
4 | | -- **Deployment**: Standard HTTP server OR AWS Lambda (both supported) |
5 | | -- **GitHub**: GitHub App required (JWT + installation token authentication) |
6 | | -- **Features**: Okta group sync + PR compliance monitoring (both optional) |
| 4 | +- **Deployment**: HTTP server (container, K8s, VM) or AWS Lambda; both serve the same router |
| 5 | +- **GitHub**: GitHub App (JWT + installation tokens); created by hand or with the github-app-setup-go installer |
| 6 | +- **Features**: Okta group sync, PR compliance monitoring, stale security alert reports, Slack notifications (all optional) |
7 | 7 | - **Entry points**: |
8 | | - - `cmd/server/main.go` - Standard HTTP server (VPS, container, K8s) |
9 | | - - `cmd/lambda/main.go` - Lambda adapter (API Gateway + EventBridge) |
10 | | - - `cmd/verify/main.go` - Integration tests with HTTP mock servers |
| 8 | + - `cmd/server/main.go` - HTTP server with graceful shutdown |
| 9 | + - `cmd/lambda/` - Lambda adapter: API Gateway HTTP API (2.0) requests and EventBridge scheduled events |
11 | 10 | - `cmd/sample/main.go` - **DO NOT RUN** (requires live credentials) |
12 | 11 | - **Packages**: |
13 | | - - `internal/app/` - Core logic and unified request handling via |
14 | | - `HandleRequest()` (no AWS dependencies) |
15 | | - - `internal/github/` - API client, webhooks, PR checks, team mgmt, auth |
16 | | - - `internal/okta/` - API client, group sync |
17 | | - - `internal/notifiers/` - Slack formatting for events and reports |
18 | | - - `internal/errors/` - Sentinel errors |
| 12 | + - `internal/service/` - composition root shared by both entry points: github-app-setup-go runtime (load, retry, reload), health and installer routes, swaps the app on reload |
| 13 | + - `internal/app/` - the application: builds clients from config, router (`/webhooks`, admin, `/scheduled/{action}`), feature handlers |
| 14 | + - `internal/config/` - reads and validates all configuration; resolves SSM parameter ARNs |
| 15 | + - `internal/domain/` - shared types, interfaces, and error kinds; imports no internal package |
| 16 | + - `internal/github/client/` - GitHub API client: auth, PR compliance, teams, security alerts |
| 17 | + - `internal/github/webhooks/` - webhook signature validation and event parsing |
| 18 | + - `internal/okta/` - Okta API client: groups and members |
| 19 | + - `internal/teamsync/` - Okta group to GitHub team sync and orphaned user detection |
| 20 | + - `internal/notifiers/` - Slack messages |
| 21 | + - `internal/setup/` - GitHub App manifest and installer credential store |
| 22 | + - `internal/fakes/` - in-memory domain fakes for tests |
| 23 | +- **Tests**: unit tests beside each package; `e2e/` runs `fixtures/scenarios.json` against the real service with mock GitHub, Okta, and Slack servers |
19 | 24 |
|
20 | 25 | ## Build & Test |
21 | | -- **Build server**: `make build-server` (creates `dist/server`) |
| 26 | +- **List targets**: `make help` |
| 27 | +- **Build server**: `make build` (creates `dist/github-ops-app`) |
22 | 28 | - **Build Lambda**: `make build-lambda` (creates `dist/bootstrap`) |
23 | | -- **Run server locally**: `make server` |
| 29 | +- **Run server locally**: `make run` (reads `.env`) |
24 | 30 | - **Test all**: `make test` (runs with `-race -count=1`) |
25 | | -- **Test single package**: `go test -race -count=1 ./internal/github` |
26 | | -- **Test single function**: `go test -race -count=1 ./internal/okta -run |
27 | | - TestGroupSync` |
28 | | -- **Integration tests**: `make test-verify` (offline tests using HTTP mock |
29 | | - servers) |
30 | | -- **Verbose integration tests**: `make test-verify-verbose` (shows all HTTP |
31 | | - requests during testing) |
32 | | -- **Lint**: `go vet ./...` and `gofmt -l .` |
| 31 | +- **Test single package**: `go test -race -count=1 ./internal/github/client` |
| 32 | +- **Test single function**: `go test -race -count=1 ./internal/teamsync -run TestSync_NamedGroup` |
| 33 | +- **End-to-end scenarios**: `make test-e2e` |
| 34 | +- **Lint**: `make lint` (golangci-lint), `make fmt` |
| 35 | +- **Vulnerabilities**: `make vuln` (govulncheck) |
| 36 | +- **Everything CI runs**: `make check` |
33 | 37 |
|
34 | | -IMPORTANT: DO NOT run `go run cmd/sample/main.go` as it requires live |
35 | | -credentials and makes real API calls to GitHub/Okta/Slack. |
| 38 | +IMPORTANT: DO NOT run `go run ./cmd/sample` or `make sample` as they require |
| 39 | +live credentials and make real API calls to GitHub/Okta/Slack. |
36 | 40 |
|
37 | 41 | ## Code Style |
38 | | -- **Imports**: stdlib, blank line, third-party, local (e.g., `internal/`) |
39 | | -- **Naming**: `PascalCase` exports, `camelCase` private, `ALL_CAPS` env vars (prefixed `APP_`) |
40 | | -- **Structs**: define types in package; constructors as `New()` or `NewTypeName()`; methods public (PascalCase) |
41 | | -- **Formatting**: `gofmt` (tabs for indentation) |
42 | | -- **Comments**: rare, lowercase, short; prefer self-documenting code |
43 | | -- **Error handling**: return errors up stack; wrap with `fmt.Errorf` (see Error Handling below) |
| 42 | +- **Imports**: stdlib, blank line, third-party, blank line, local (`github.com/cruxstack/github-ops-app/...`); `make fmt` enforces it |
| 43 | +- **Naming**: `PascalCase` exports, `camelCase` private |
| 44 | +- **Constructors**: `New(Options)` or `New(cfg, ...Option)`; constructors make no network calls |
| 45 | +- **Comments**: lowercase inline comments; doc comments are full sentences |
| 46 | +- **Logging**: `log/slog` with key-value pairs (`"team", slug`), snake_case keys, `*Context` methods where a context exists |
| 47 | +- **Formatting**: `gofmt` and `goimports` (tabs for indentation) |
| 48 | + |
| 49 | +## Configuration |
| 50 | +- All variables are read in `internal/config`; nothing else calls `os.Getenv` |
| 51 | +- App settings use the `APP_` prefix; installer settings keep github-app-setup-go's names (`GITHUB_APP_INSTALLER_ENABLED`, `STORAGE_MODE`, ...) |
| 52 | +- Validate at load time and report every problem at once; invalid or partial configuration is an error, never a silent default |
| 53 | +- Renamed variables go in `removed` in `config.go`, so setting the old name is an error |
| 54 | +- Any `APP_` value may be an SSM parameter ARN; installer variables are read as github-app-setup-go reads them |
44 | 55 |
|
45 | 56 | ## Error Handling |
46 | 57 |
|
47 | | -### Error Message Format |
| 58 | +### Error Messages |
48 | 59 | - **Style**: lowercase, action-focused, concise |
49 | 60 | - **Pattern**: `"failed to {action} {object}: {context}"` |
50 | 61 | - **Always include**: specific identifiers (PR numbers, team names, IDs) |
51 | 62 | - **Examples**: |
52 | | - - ✅ `"failed to fetch pr #123 from owner/repo: %w"` |
53 | | - - ✅ `"failed to create team 'engineers' in org 'myorg': %w"` |
54 | | - - ✅ `"required check 'ci' did not pass"` |
55 | | - - ❌ `"PR is nil"` (no context) |
56 | | - - ❌ `"Failed to Get Team"` (capitalized, generic) |
| 63 | + - ✅ `fmt.Errorf("failed to fetch pr #%d from %s/%s: %w", n, owner, repo, err)` |
| 64 | + - ✅ `fmt.Errorf("failed to create team '%s' in org '%s': %w", name, org, err)` |
| 65 | + - ❌ `errors.New("PR is nil")` (no context) |
| 66 | + - ❌ `errors.New("Failed to Get Team")` (capitalized, generic) |
57 | 67 |
|
58 | | -### Error Wrapping |
59 | | -- Use `github.com/cockroachdb/errors` package for all error handling |
60 | | -- **Wrap errors**: `errors.Wrap(err, "context")` or `errors.Wrapf(err, |
61 | | - "failed to sync team '%s'", teamName)` |
62 | | -- **Create new errors**: `errors.New("message")` or `errors.Newf("error: |
63 | | - %s", context)` |
64 | | -- Automatically captures stack traces for debugging Lambda issues |
65 | | -- Preserve original error context while adding specific details |
| 68 | +### Wrapping |
| 69 | +- Use the standard library: `fmt.Errorf("...: %w", err)`, `errors.Is`, `errors.As`, `errors.Join` |
| 70 | +- Return errors up the stack with context; don't log and return the same error |
66 | 71 |
|
67 | | -### Sentinel Errors |
68 | | -- Define common errors in `internal/errors/errors.go` |
69 | | -- Each sentinel error is marked with a domain type (ValidationError, |
70 | | - AuthError, APIError, ConfigError) |
71 | | -- Domain markers enable error classification and monitoring |
72 | | -- Use `errors.Is()` to check for sentinel errors in tests |
73 | | -- Use `errors.HasType()` to check for error domains |
74 | | -- Examples: `ErrMissingPRData`, `ErrInvalidSignature`, `ErrClientNotInit` |
| 72 | +### Error Kinds |
| 73 | +- `internal/domain/errors.go` defines kinds (`ErrValidation`, `ErrAuth`, `ErrAPI`, `ErrConfig`) and sentinels that belong to a kind (`ErrInvalidSignature`, `ErrTeamNotFound`, ...) |
| 74 | +- `errors.Is(err, domain.ErrAuth)` matches any auth sentinel, through wrapping |
| 75 | +- The HTTP layer maps kinds to status codes in `writeDomainError` and never returns error details to clients |
75 | 76 |
|
76 | | -### Stack Traces |
77 | | -- Automatically captured when wrapping errors with cockroachdb/errors |
78 | | -- No performance overhead unless error is formatted |
79 | | -- Critical for debugging serverless Lambda executions |
80 | | -- Use `errors.WithDetailf()` to add structured context to auth/API errors |
81 | | - |
82 | | -### Validation |
83 | | -- Validate at parse time, not during processing |
84 | | -- Webhook events validated in `ParseXxxEvent()` functions |
85 | | -- Return detailed validation errors immediately |
86 | | -- Prevents nil pointer issues downstream |
87 | | - |
88 | | -### Error Logging vs Returning |
| 77 | +### Logging vs Returning |
89 | 78 | - **Fatal errors** (config, init): return immediately |
90 | | -- **Recoverable errors** (individual items in batch): collect and continue |
91 | | -- **Optional features** (notifications): log only, don't fail parent operation |
92 | | -- Lambda handlers: log detailed errors, return sanitized messages to client |
93 | | - |
94 | | -### Batch Operation Errors |
95 | | -- Collect errors in result structs (e.g., `SyncReport.Errors`) |
96 | | -- Continue processing remaining items |
97 | | -- Return aggregated results with partial success |
98 | | -- Helper methods: `HasErrors()`, `HasChanges()` |
99 | | - |
100 | | -## Authentication & Integration |
101 | | - |
102 | | -### GitHub |
103 | | -- **Required**: GitHub App (JWT + installation tokens, automatic rotation) |
104 | | -- **Auth flow**: JWT signed with private key → exchange for installation token → cached with auto-refresh |
105 | | -- **Webhooks**: HMAC-SHA256 signature verification |
106 | | - |
107 | | -### Okta |
108 | | -- OAuth 2.0 with private key authentication |
109 | | -- **Required scopes**: `okta.groups.read` and `okta.users.read` |
110 | | -- Sync uses slug-based GitHub Teams API |
111 | | - |
112 | | -### Slack |
113 | | -- Optional notifications for PR events and sync reports |
114 | | -- Configuration in `internal/notifiers/` |
| 79 | +- **Recoverable errors** (one user, team, or alert source in a batch): collect in the result (`SyncReport.Errors`, `SecurityAlertsReport.Errors`) and continue |
| 80 | +- **Optional features** (notifications): log, don't fail the parent operation |
| 81 | + |
| 82 | +### Batch Operations |
| 83 | +- Collect errors in result structs and continue with the remaining items |
| 84 | +- Never remove team members based on incomplete data: a failed Okta read skips the group, the safety threshold caps removals, and child-team members are never removed from a parent |
| 85 | +- Never report orphaned users from incomplete data: detection is skipped unless every rule's team was found |
| 86 | + |
| 87 | +## Security |
| 88 | +- Webhooks are always signature-checked; there is no unsigned mode |
| 89 | +- Admin endpoints require `APP_ADMIN_TOKEN` (constant-time compare) and are disabled without it |
| 90 | +- Only events from `APP_GITHUB_ORG` are acted on |
| 91 | +- Escape GitHub and Okta text in Slack mrkdwn with `escape`/`link` in `internal/notifiers` |
| 92 | +- Validate GitHub logins (`domain.IsValidGitHubLogin`) before putting them in API paths |
| 93 | +- Auth headers are only sent to the configured GitHub API host |
115 | 94 |
|
116 | 95 | ## Markdown Style |
117 | 96 | - **Line length**: Max 80 characters for prose text |
|
0 commit comments