-
Notifications
You must be signed in to change notification settings - Fork 6
docs: add AGENTS.md and pull-requests skill #245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,4 @@ | |
|
|
||
| # Project specific | ||
| example/aibridge.db | ||
|
|
||
| AGENTS.local.md | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| --- | ||
| name: pull-requests | ||
| description: > | ||
| Guide for creating, updating, and following up on pull requests in the | ||
| aibridge repository. Use when asked to open a PR, update a PR, rewrite | ||
| a PR description, or follow up on CI/check failures. | ||
| --- | ||
|
|
||
| # Pull Request Skill | ||
|
|
||
| ## When to Use This Skill | ||
|
|
||
| Use this skill when asked to: | ||
|
|
||
| - Create a pull request for the current branch. | ||
| - Update an existing PR branch or description. | ||
| - Rewrite a PR body. | ||
| - Follow up on CI or check failures for an existing PR. | ||
|
|
||
| ## References | ||
|
|
||
| - PR conventions and local validation: `AGENTS.md` (Essential Commands and | ||
| Commit Style sections) | ||
|
|
||
| ## Lifecycle Rules | ||
|
|
||
| 1. **Check for an existing PR** before creating a new one: | ||
|
|
||
| ``` | ||
| gh pr list --head "$(git branch --show-current)" --author @me --json number --jq '.[0].number // empty' | ||
| ``` | ||
|
|
||
| If that returns a number, update that PR. If it returns empty output, | ||
| create a new one. | ||
|
|
||
| 2. **Check you are not on main.** If the current branch is `main` or `master`, | ||
| create a feature branch before doing PR work. | ||
|
|
||
| 3. **Default to draft.** Use `gh pr create --draft` unless the user explicitly | ||
| asks for ready-for-review. | ||
|
|
||
| 4. **Keep description aligned with the full diff.** Re-read the diff against | ||
| the base branch before writing or updating the title and body. Describe the | ||
| entire PR diff, not just the last commit. | ||
|
|
||
| 5. **Never auto-merge.** Do not merge or mark ready for review unless the user | ||
| explicitly asks. | ||
|
|
||
| 6. **Never push to main or master.** | ||
|
|
||
| ## PR Title Format | ||
|
|
||
| Follow the commit style from `AGENTS.md`: | ||
|
|
||
| ``` | ||
| type(scope): message | ||
| ``` | ||
|
|
||
| Examples: | ||
| - `feat(mcp): add tool allowlist filtering` | ||
| - `fix(intercept/messages): handle empty streaming chunks` | ||
| - `refactor(provider): extract common auth logic` | ||
|
|
||
| ## PR Description | ||
|
|
||
| - Start with a one-line summary of **what** and **why**. | ||
| - List key changes as bullet points. | ||
| - Note any breaking changes or migration steps. | ||
| - Do not fabricate or embellish — describe only what the diff contains. | ||
|
|
||
| ## CI / Checks Follow-up | ||
|
|
||
| **Always watch CI checks after pushing.** Do not push and walk away. | ||
|
|
||
| After pushing: | ||
|
|
||
| - Monitor CI with `gh pr checks <PR_NUMBER> --watch`. | ||
| - Use `gh pr view <PR_NUMBER> --json statusCheckRollup` for programmatic check | ||
| status. | ||
|
|
||
| If checks fail: | ||
|
|
||
| 1. Find the failed run ID from the `gh pr checks` output. | ||
| 2. Read the logs with `gh run view <run-id> --log-failed`. | ||
| 3. Fix the problem locally. | ||
| 4. Run `make test` and `make fmt` before pushing the fix. | ||
| 5. Push the fix. | ||
|
|
||
| ## Pre-Push Validation | ||
|
|
||
| Before pushing, always run: | ||
|
|
||
| ```bash | ||
| make fmt | ||
| make test | ||
| ``` | ||
|
|
||
| ## What Not to Do | ||
|
|
||
| - Do not reference or call helper scripts that do not exist in this repository. | ||
| - Do not auto-merge or mark ready for review without explicit user request. | ||
| - Do not push to `origin/main` or `origin/master`. | ||
| - Do not skip local validation before pushing. | ||
| - Do not fabricate or embellish PR descriptions. | ||
| - Do not use `--no-verify` on git operations. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this repo has any git hooks, so this will be a no-op. Nevertheless, good to have if that changes. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| # AI Agent Guidelines for aibridge | ||
|
|
||
| > For local overrides, create `AGENTS.local.md` (gitignored). | ||
|
|
||
| You are an experienced, pragmatic software engineer. Simple solutions | ||
| over clever ones. Readability is a primary concern. | ||
|
|
||
| ## Tone & Relationship | ||
|
|
||
| We're colleagues — push back on bad ideas and speak up when something | ||
| doesn't make sense. Honesty over agreeableness. | ||
|
|
||
| - Disagree when I'm wrong — act as a critical peer reviewer. | ||
| - Call out bad ideas, unreasonable expectations, and mistakes. | ||
| - **Ask for clarification** instead of assuming. Say when you don't know something. | ||
| - Architectural decisions require discussion; routine fixes do not. | ||
|
|
||
| ## Foundational Rules | ||
|
|
||
| - Doing it right is better than doing it fast. | ||
| - YAGNI — don't add features we don't need right now. | ||
| - Make the smallest reasonable changes to achieve the goal. | ||
| - Reduce code duplication, even if it takes extra effort. | ||
| - Match the style of surrounding code — consistency within a file matters. | ||
| - Fix bugs immediately when you find them. | ||
|
|
||
| ## Essential Commands | ||
|
|
||
| | Task | Command | Notes | | ||
| | ----------- | ---------------------- | --------------------------------- | | ||
| | Test | `make test` | All tests, no race detector | | ||
| | Test (race) | `make test-race` | CGO_ENABLED=1, use for CI | | ||
| | Coverage | `make coverage` | Prints summary to stdout | | ||
| | Format | `make fmt` | gofumpt; single file: `make fmt FILE=path` | | ||
| | Mocks | `make mocks` | Regenerate from `mcp/api.go` | | ||
|
|
||
| **Always use these commands** instead of running `go test` or `gofumpt` directly. | ||
|
|
||
| ## Code Navigation | ||
|
|
||
| Use LSP tools (go to definition, find references, hover) **before** resorting to grep. | ||
| This codebase has 90+ Go files across multiple packages — LSP is faster and more accurate. | ||
|
|
||
| ## Architecture Overview | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section is already covered in the repo readme: https://github.com/coder/aibridge/blob/main/README.md#architecture |
||
|
|
||
| AI Bridge is a smart gateway that sits between AI clients (Claude Code, Cursor, | ||
| etc.) and upstream providers (Anthropic, OpenAI). It intercepts all AI traffic | ||
| to provide centralized authn/z, auditing, token attribution, and MCP tool | ||
| administration. It runs as part of `coderd` (the Coder control plane) — users | ||
| authenticate with their Coder session tokens. | ||
|
|
||
| ``` | ||
| ┌─────────────┐ ┌──────────────────────────────────────────┐ | ||
| │ AI Client │ │ aibridge │ | ||
| │ (Claude Code,│────▶│ RequestBridge (http.Handler) │ | ||
| │ Cursor) │ │ ├── Provider (Anthropic/OpenAI) │ | ||
| └─────────────┘ │ ├── Interceptor (streaming/blocking) │ | ||
| │ ├── Recorder (tokens, prompts, tools) │ | ||
| │ └── MCP Proxy (tool injection) │ | ||
| └──────────────┬───────────────────────────┘ | ||
| │ | ||
| ▼ | ||
| ┌──────────────┐ | ||
| │ Upstream API │ | ||
| │ (Anthropic, │ | ||
| │ OpenAI) │ | ||
| └──────────────┘ | ||
| ``` | ||
|
|
||
| Key packages: | ||
| - `intercept/` — request/response interception, per-provider subdirs (`messages/`, `responses/`, `chatcompletions/`) | ||
| - `provider/` — upstream provider definitions (Anthropic, OpenAI, Copilot) | ||
| - `mcp/` — MCP protocol integration | ||
| - `circuitbreaker/` — circuit breaker for upstream calls | ||
| - `context/` — request-scoped context helpers | ||
| - `internal/integrationtest/` — integration tests with mock upstreams | ||
|
|
||
| ## Go Patterns | ||
|
|
||
| - Follow the [Uber Go Style Guide](https://github.com/uber-go/guide/blob/master/style.md). | ||
| - Use `gofumpt` for formatting (enforced by `make fmt`). | ||
| - Prefer table-driven tests. | ||
| - **Never use `time.Sleep` in tests** — use `github.com/coder/quartz` or channels/contexts for synchronization. | ||
| - Use unique identifiers in tests: `fmt.Sprintf("test-%s-%d", t.Name(), time.Now().UnixNano())`. | ||
| - Test observable behavior, not implementation details. | ||
|
|
||
| ## Streaming Code | ||
|
|
||
| This codebase heavily uses SSE streaming. When modifying interceptors: | ||
| - Always handle both blocking and streaming paths. | ||
| - Test with `*_test.go` files in the same package — they cover edge cases for chunked responses. | ||
| - Be careful with goroutine lifecycle — ensure proper cleanup on context cancellation. | ||
|
|
||
| ## Commit Style | ||
|
|
||
| ``` | ||
| type(scope): message | ||
| ``` | ||
|
|
||
| - `scope` = real package path (e.g., `intercept/messages`, `provider`, `circuitbreaker`) | ||
| - Comments: full sentences, max 80 chars, explain **why** not what. | ||
|
|
||
| ## Do NOT | ||
|
|
||
| - Rewrite comments or refactor code that isn't related to your task. | ||
| - Remove context from error messages. | ||
| - Use `--no-verify` on git operations. | ||
| - Add `//nolint` without a justification comment. | ||
| - Introduce new dependencies without discussion. | ||
|
|
||
| ## Common Pitfalls | ||
|
|
||
| | Problem | Fix | | ||
| | ------- | --- | | ||
| | Race in streaming tests | Use `t.Cleanup()` and proper synchronization, never `time.Sleep` | | ||
| | Mock not updated | Run `make mocks` after changing `mcp/api.go` | | ||
| | Formatting failures | Run `make fmt` before committing | | ||
| | `retract` directive in go.mod | Don't remove — it's intentional (v1.0.8 conflict marker) | | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: newline missing