goose/.github/copilot-instructions.md
Bradley Axen b18120bec3
Some checks failed
Canary / Prepare Version (push) Waiting to run
Canary / build-cli (push) Blocked by required conditions
Canary / Upload Install Script (push) Blocked by required conditions
Canary / bundle-desktop (push) Blocked by required conditions
Canary / bundle-desktop-linux (push) Blocked by required conditions
Canary / bundle-desktop-windows (push) Blocked by required conditions
Canary / Release (push) Blocked by required conditions
CI / changes (push) Waiting to run
CI / Check Rust Code Format (push) Blocked by required conditions
CI / Build and Test Rust Project (push) Blocked by required conditions
CI / Lint Rust Code (push) Blocked by required conditions
CI / Check OpenAPI Schema is Up-to-Date (push) Blocked by required conditions
CI / Test and Lint Electron Desktop App (push) Blocked by required conditions
Live Provider Tests / Smoke Tests (Code Execution) (push) Blocked by required conditions
Live Provider Tests / check-fork (push) Waiting to run
Live Provider Tests / changes (push) Blocked by required conditions
Live Provider Tests / Build Release Binary (push) Blocked by required conditions
Live Provider Tests / Smoke Tests (push) Blocked by required conditions
Publish Docker Image / docker (push) Waiting to run
Scorecard supply-chain security / Scorecard analysis (push) Waiting to run
Deploy Documentation / deploy (push) Has been cancelled
Publish Ask AI Bot Docker Image / docker (push) Has been cancelled
Remove clippy too_many_lines lint and decompose long functions (#7064)
2026-02-07 04:10:24 +00:00

4.7 KiB

GitHub Copilot Code Review Instructions

Review Philosophy

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

Security & Safety

  • Unsafe code blocks without justification
  • Command injection risks (shell commands, user input)
  • Path traversal vulnerabilities
  • Credential exposure or hardcoded secrets
  • Missing input validation on external data
  • Improper error handling that could leak sensitive info

Correctness Issues

  • Logic errors that could cause panics or incorrect behavior
  • Race conditions in async code
  • Resource leaks (files, connections, memory)
  • Off-by-one errors or boundary conditions
  • Incorrect error propagation (using unwrap() inappropriately)
  • Optional types that don't need to be optional
  • Booleans that should default to false but are set as optional
  • Error context that doesn't add useful information (e.g., .context("Failed to do X") when error already says it failed)
  • Overly defensive code that adds unnecessary checks
  • Unnecessary comments that just restate what the code already shows (remove them)

Architecture & Patterns

  • Code that violates existing patterns in the codebase
  • Missing error handling (should use anyhow::Result)
  • Async/await misuse or blocking operations in async contexts
  • Improper trait implementations

No Prerelease Docs

  • If the PR contains both code changes to features/functionality AND updates in /documentation: Documentation updates must be separated to keep public docs in sync with released versions. Either mark new topics with unlisted: true or remove/hide the documentation.

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
  • Naming convention: In documentation/docs and documentation/blog, always refer to the project as "goose" (lowercase), never "Goose" (even at the start of sentences)

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
  • cargo clippy --all-targets -- -D warnings - 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)

Do not comment on:

  • Style/formatting - CI handles this (rustfmt, prettier)
  • Clippy warnings - CI handles this (clippy)
  • 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

When you identify an issue:

  1. State the problem (1 sentence)
  2. Why it matters (1 sentence, only if not obvious)
  3. Suggested fix (code snippet or specific action)

Example:

This could panic if the vector is empty. Consider using `.get(0)` or add a length check.

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.