teach copilot our CI (#5672)

This commit is contained in:
Angie Jones
2025-11-11 09:47:06 -06:00
committed by GitHub
parent 5110d32142
commit fb68966b1e
+47 -20
View File
@@ -4,7 +4,6 @@
- Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists - Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists
- Be concise: one sentence per comment when possible - Be concise: one sentence per comment when possible
- Focus on actionable feedback, not observations - Focus on actionable feedback, not observations
- Skip comments on style that clippy/rustfmt will catch
- When reviewing text, only comment on clarity issues if the text is genuinely confusing or could lead to errors. "Could be clearer" is not the same as "is confusing" - stay silent unless HIGH confidence it will cause problems - When reviewing text, only comment on clarity issues if the text is genuinely confusing or could lead to errors. "Could be clearer" is not the same as "is confusing" - stay silent unless HIGH confidence it will cause problems
## Priority Areas (Review These) ## Priority Areas (Review These)
@@ -35,17 +34,55 @@
- Async/await misuse or blocking operations in async contexts - Async/await misuse or blocking operations in async contexts
- Improper trait implementations - Improper trait implementations
## Project-Specific Context
- This is a Rust project using cargo workspaces
- Core crates: `goose` (agent logic), `goose-cli` (CLI), `goose-server` (backend), `goose-mcp` (MCP servers)
- Error handling: Use `anyhow::Result`, not `unwrap()` in production code
- Async runtime: tokio
- See HOWTOAI.md for AI-assisted code standards
- MCP protocol implementations require extra scrutiny
## CI Pipeline Context
**Important**: You review PRs immediately, before CI completes. Do not flag issues that CI will catch.
### What Our CI Checks (`.github/workflows/ci.yml`)
**Rust checks:**
- `cargo fmt --check` - Code formatting (rustfmt)
- `cargo test --jobs 2` - All tests
- `./scripts/clippy-lint.sh` - Linting (clippy)
- `just check-openapi-schema` - OpenAPI schema validation
**Desktop app checks:**
- `npm ci` - Fresh dependency install (in `ui/desktop/`)
- `npm run lint:check` - ESLint + Prettier
- `npm run test:run` - Vitest tests
**Setup steps CI performs:**
- Installs system dependencies (libdbus, gnome-keyring, libxcb)
- Activates hermit environment (`source bin/activate-hermit`)
- Caches Cargo and npm dependencies
- Runs `npm ci` before any npm scripts (ensures all packages are installed)
**Key insight**: Commands like `npx` check local `node_modules` first, which CI installs via `npm ci`. Don't flag these as broken unless you can explain why CI setup wouldn't handle it.
## Skip These (Low Value) ## Skip These (Low Value)
- Style issues (rustfmt handles this) Do not comment on:
- Clippy warnings (CI catches these) - **Style/formatting** - CI handles this (rustfmt, prettier)
- Minor naming suggestions unless truly confusing - **Clippy warnings** - CI handles this (clippy-lint.sh)
- Obvious code that doesn't need explanation - **Test failures** - CI handles this (full test suite)
- Suggestions to add comments for self-documenting code - **Missing dependencies** - CI handles this (npm ci will fail)
- Refactoring suggestions unless there's a clear bug or maintainability issue - **Minor naming suggestions** - unless truly confusing
- Listing multiple potential issues in one comment (choose the single most critical issue) - **Suggestions to add comments** - for self-documenting code
- Suggestions to add logging statements, unless for errors or security events (the codebase needs less logging, not more) - **Refactoring suggestions** - unless there's a clear bug or maintainability issue
- **Multiple issues in one comment** - choose the single most critical issue
- **Logging suggestions** - unless for errors or security events (the codebase needs less logging, not more)
- **Pedantic accuracy in text** - unless it would cause actual confusion or errors. No one likes a reply guy
## Response Format ## Response Format
@@ -59,16 +96,6 @@ Example:
This could panic if the vector is empty. Consider using `.get(0)` or add a length check. This could panic if the vector is empty. Consider using `.get(0)` or add a length check.
``` ```
## Project-Specific Context
- This is a Rust project using cargo workspaces
- Core crates: `goose` (agent logic), `goose-cli` (CLI), `goose-server` (backend), `goose-mcp` (MCP servers)
- Error handling: Use `anyhow::Result`, not `unwrap()` in production code
- Async runtime: tokio
- All code must pass: `cargo fmt`, `./scripts/clippy-lint.sh`, and tests
- See HOWTOAI.md for AI-assisted code standards
- MCP protocol implementations require extra scrutiny
## When to Stay Silent ## When to Stay Silent
If you're uncertain whether something is an issue, don't comment. False positives create noise and reduce trust in the review process. If you're uncertain whether something is an issue, don't comment. False positives create noise and reduce trust in the review process.