Skip to content

teach copilot our CI #5672

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.

Sign up for GitHub

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

Merged
angiejones merged 3 commits into from
Nov 11, 2025
Merged

teach copilot our CI #5672

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6cdf8f3
teach copilot our CI
angiejones Nov 11, 2025
81e10ad
remove the reply guy act
angiejones Nov 11, 2025
3b80ba7
Apply suggestion from @angiejones
angiejones Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 47 additions & 20 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -35,17 +34,55 @@
- Async/await misuse or blocking operations in async contexts
- Improper trait implementations

## 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)

## 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)
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation lists simplified package names (libdbus, libxcb) but CI installs the full -dev package names (libdbus-1-dev, libxcb1-dev). Update to match the actual CI for accuracy: libdbus-1-dev, gnome-keyring, libxcb1-dev.

Suggested change
- Installs system dependencies (libdbus, gnome-keyring, libxcb)
- Installs system dependencies (`libdbus-1-dev`, `gnome-keyring`, `libxcb1-dev`)

Copilot uses AI. Check for mistakes.
- 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)

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

Expand All @@ -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.