diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5cc6ed8f5e..cdb5a4db86 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -4,7 +4,6 @@ - Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists - Be concise: one sentence per comment when possible - 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 ## Priority Areas (Review These) @@ -35,17 +34,55 @@ - Async/await misuse or blocking operations in async contexts - 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) -- Style issues (rustfmt handles this) -- Clippy warnings (CI catches these) -- Minor naming suggestions unless truly confusing -- Obvious code that doesn't need explanation -- Suggestions to add comments for self-documenting code -- Refactoring suggestions unless there's a clear bug or maintainability issue -- Listing multiple potential issues in one comment (choose the single most critical issue) -- Suggestions to add logging statements, unless for errors or security events (the codebase needs less logging, not more) - +Do not comment on: +- **Style/formatting** - CI handles this (rustfmt, prettier) +- **Clippy warnings** - CI handles this (clippy-lint.sh) +- **Test failures** - CI handles this (full test suite) +- **Missing dependencies** - CI handles this (npm ci will fail) +- **Minor naming suggestions** - unless truly confusing +- **Suggestions to add comments** - for self-documenting code +- **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 @@ -59,16 +96,6 @@ Example: 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 If you're uncertain whether something is an issue, don't comment. False positives create noise and reduce trust in the review process.