roost.tools
Mirrored from GitHub
github.com/roostorg/osprey
Define AI Code review configuration and guidelines (#279)
author
Juan Mrad
committer
GitHub
date
(May 21, 2026, 12:17 PM -0500)
commit
615ac019
615ac0196efc0fc5aa67a54b6316d355216fdd3b
parent
7aaa7dd5
7aaa7dd5c492890861592cf3596c0f5ca9323bcf
+213
+137
.coderabbit.yaml
+137
.coderabbit.yaml
···
1
+
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
2
+
#
3
+
# Review rules live in `.github/copilot-instructions.md` and `AGENTS.md`
4
+
# CodeRabbit picks both up automatically. This file only carries settings that
5
+
# differ from CodeRabbit's defaults.
6
+
7
+
reviews:
8
+
profile: chill
9
+
request_changes_workflow: false
10
+
high_level_summary: true
11
+
poem: false
12
+
review_status: true
13
+
collapse_walkthrough: true
14
+
15
+
auto_review:
16
+
enabled: true
17
+
drafts: false
18
+
base_branches:
19
+
- main
20
+
ignore_usernames:
21
+
- 'dependabot[bot]'
22
+
- 'dependabot-preview[bot]'
23
+
labels:
24
+
- '!dependabot'
25
+
26
+
path_filters:
27
+
# Generated protobuf bindings — regenerate via ./gen-protos.sh, never
28
+
# hand-edit (AGENTS.md > Architecture).
29
+
- '!osprey_rpc/src/osprey/rpc/**/*_pb2*.py'
30
+
- '!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyi'
31
+
- '!osprey_coordinator/src/proto/**'
32
+
# Build artifacts and caches.
33
+
- '!**/node_modules/**'
34
+
- '!**/dist/**'
35
+
- '!**/build/**'
36
+
- '!**/target/**'
37
+
- '!**/__pycache__/**'
38
+
- '!**/.venv/**'
39
+
- '!**/*.snap'
40
+
41
+
# Cross-cutting rules live in .github/copilot-instructions.md.
42
+
path_instructions:
43
+
- path: 'osprey_worker/src/osprey/worker/ui_api/**/*.py'
44
+
instructions: |
45
+
Flask HTTP API on port 5004. Verify each view enforces authentication
46
+
and that any caller-supplied IDs are authorization-checked, not just
47
+
existence-checked (IDOR). Flag string-built SQL or shell commands —
48
+
SQLAlchemy bound parameters and `subprocess` arg arrays (no
49
+
`shell=True`) are required. Validate request bodies and query params
50
+
(Pydantic preferred). Error responses must not leak Python stack
51
+
traces or internal exception messages.
52
+
53
+
- path: 'osprey_worker/src/osprey/worker/sinks/**/*.py'
54
+
instructions: |
55
+
Output sinks emit verdicts/effects to external systems (Kafka, HTTP,
56
+
etc.). Flag sink writes that don't bound retries or backoff, that log
57
+
full request/response bodies or raw Kafka payloads (PII risk), or
58
+
that swallow exceptions without a metric, log, or rethrow.
59
+
60
+
- path: 'osprey_worker/src/osprey/worker/adaptor/**/*.py'
61
+
instructions: |
62
+
Pluggy plugin manager and hookspecs (`register_udfs`,
63
+
`register_output_sinks`, `register_labels_service_or_provider`). New
64
+
or renamed hookspecs change the plugin contract — surface signature
65
+
changes that would break `example_plugins/` or downstream consumers.
66
+
Generic-name UDFs collide silently; flag duplicates.
67
+
68
+
- path: 'osprey_ui/src/**/*.{ts,tsx}'
69
+
instructions: |
70
+
- XSS: flag `dangerouslySetInnerHTML`, `innerHTML`, `document.write`,
71
+
`javascript:` URLs, and unsanitized `href`/`src` from user input.
72
+
Prefer `textContent`; sanitize with DOMPurify when raw HTML is
73
+
unavoidable.
74
+
- Token storage: auth tokens belong in HttpOnly, Secure, SameSite
75
+
cookies — flag `localStorage` or `sessionStorage` use for tokens.
76
+
- Open redirects: redirecting to a user-supplied URL without an
77
+
allowlist is risky.
78
+
79
+
- path: 'osprey_coordinator/src/**/*.rs'
80
+
instructions: |
81
+
- `unwrap()` / `expect()` / `panic!` on user-driven or RPC paths
82
+
turn a bad input into a process crash; prefer `?` or explicit
83
+
error mapping.
84
+
- `unsafe` blocks deserve a justifying comment and the tightest
85
+
possible scope.
86
+
- Holding a `std::sync::Mutex` guard across `.await` deadlocks the
87
+
tokio runtime; long-lived `tokio::sync` guards can starve other
88
+
tasks. Flag them.
89
+
- Spawned tasks should be cancellation-safe — surface anything that
90
+
will leak on shutdown.
91
+
92
+
- path: 'proto/osprey/rpc/**/*.proto'
93
+
instructions: |
94
+
gRPC contract consumed by both Python workers and the Rust
95
+
coordinator. Removing or renaming fields, changing field numbers, or
96
+
changing tag types is a breaking change — flag it and ask for a
97
+
migration plan. Bindings must be regenerated with `./gen-protos.sh`
98
+
in the same PR.
99
+
100
+
- path: 'example_plugins/**/*.py'
101
+
instructions: |
102
+
Reference plugins. Per AGENTS.md > Architecture no production code
103
+
belongs here — flag if production-only logic is being added.
104
+
105
+
- path: 'example_rules/**'
106
+
instructions: |
107
+
Sample SML rules and YAML config. Reference, not production — flag
108
+
if production rule logic is being added here instead of in the
109
+
consumer's own rules directory.
110
+
111
+
- path: '**/{pyproject.toml,uv.lock,Cargo.toml,Cargo.lock,package.json,package-lock.json}'
112
+
instructions: |
113
+
Dependency additions, removals, or upgrades (including transitive
114
+
bumps) require human approval for license (Apache 2.0, per
115
+
`LICENSE.md`) and CVE review per AGENTS.md > "Human-approval-required
116
+
actions". Surface every change so reviewers don't miss it. New
117
+
Python deps must also be exercised by `fawltydeps` or added to
118
+
`[tool.fawltydeps].ignore_unused` with a comment.
119
+
120
+
- path: '{docker-compose.yaml,start.sh,entrypoint.sh,**/Dockerfile,.github/workflows/publish-coordinator-image.yml,.github/workflows/release-osprey-rpc.yml,.github/workflows/mdbook.yml}'
121
+
instructions: |
122
+
AGENTS.md > "Human-approval-required actions" lists these as
123
+
restricted: release/deploy workflows, production Dockerfiles,
124
+
signing/tagging, and infra entrypoints. Flag any change here as
125
+
needing explicit human approval. Default Docker bindings are
126
+
`127.0.0.1` — flag any bind-address change.
127
+
128
+
# These overlap with checks already enforced in CI, or add style noise.
129
+
tools:
130
+
ruff:
131
+
enabled: false
132
+
prettier:
133
+
enabled: false
134
+
markdownlint:
135
+
enabled: false
136
+
languagetool:
137
+
enabled: false
+70
.github/copilot-instructions.md
+70
.github/copilot-instructions.md
···
1
+
# Code review instructions
2
+
3
+
These instructions apply to AI tools when they review pull requests in this repository, and when they answer questions about this codebase. They are guidance, not a checklist. Use judgment, prefer fewer high-signal comments over many low-signal ones, and skip points that don't apply to the diff in front of you. For human contributor guidance see [`README.md`](../README.md); for general agent rules see [`AGENTS.md`](../AGENTS.md).
4
+
5
+
## Repository at a glance
6
+
7
+
- Multi-language monorepo: Python (worker + UI API), TypeScript (UI), Rust (coordinator). Top-level modules are documented in [`AGENTS.md`](../AGENTS.md) § Architecture — `osprey_worker/`, `osprey_rpc/`, `osprey_ui/`, `osprey_coordinator/`, `proto/osprey/rpc/`, `example_plugins/`, `example_rules/`.
8
+
- Python version pinned in `.python-version`, managed with [`uv`](https://docs.astral.sh/uv/). Linted and type-checked in CI by `ruff` + `mypy` via `pre-commit`. Unused-dep enforcement via `fawltydeps`.
9
+
- TypeScript in `osprey_ui/` (React + Ant Design + Highcharts; Node version in `.github/workflows/code-quality.yml`). Formatted in CI by `prettier` (`npm run format:check`).
10
+
- Rust in `osprey_coordinator/` (stable toolchain, `tokio` + `tonic` + `etcd` + `rdkafka`). `cargo fmt` and `cargo build` gate CI; `cargo clippy -- -D warnings` and `cargo test` run advisory (`continue-on-error: true`).
11
+
- **Generated code**: `osprey_rpc/src/osprey/rpc/**/*_pb2*.py`, `*_pb2*.pyi`, and `osprey_coordinator/src/proto/` are produced by `./gen-protos.sh` from `proto/osprey/rpc/**/*.proto`. Hand-edits drift from the schema — regenerate instead. Generated files are excluded from `ruff` and `mypy` for a reason.
12
+
- Plugin system: `pluggy` with `@hookimpl_osprey` hooks (`register_udfs`, `register_output_sinks`, `register_labels_service_or_provider`). Reference patterns live in `example_plugins/src/register_plugins.py`.
13
+
- Data model conventions: Pydantic for models, SQLAlchemy for persistence (versions pinned in `pyproject.toml`).
14
+
15
+
## Scope of review — focus on quality and security
16
+
17
+
Lint and formatting are enforced in CI, so please skip:
18
+
19
+
- formatting, whitespace, indentation, quote style, or import ordering
20
+
- `ruff` / `mypy` / `prettier` / `cargo fmt` rule violations
21
+
- typos in comments or doc grammar nits
22
+
- missing docstrings on internal helpers
23
+
- subjective style preferences not codified in a project rule
24
+
25
+
If a finding would be caught by `uv run ruff check`, `uv run mypy .`, `npm run format:check` (in `osprey_ui/`), or `cargo fmt --check` (in `osprey_coordinator/`), it's redundant.
26
+
27
+
## Security (cross-cutting)
28
+
29
+
Security findings are the highest-value comments you can leave. When you spot one, name the risk concretely and suggest a fix. Areas to watch across the whole codebase:
30
+
31
+
- **Hard-coded secrets.** API keys, tokens, passwords, OAuth secrets, JWT signing keys, DB connection strings (Postgres, Druid, Kafka SASL, MinIO, Bigtable), or webhook secrets in source or committed config. `docker-compose.yaml` uses environment variables intentionally — flag any secret that leaks into committed files.
32
+
- **Injection.** String-built SQL, shell commands, file paths, HTML, or LLM prompts are usually a smell. Look for SQLAlchemy bound parameters (or `text()` with `bindparams`), `subprocess` arg arrays (no `shell=True`), context-aware HTML encoding, and a clear separation between trusted system prompts and untrusted user content.
33
+
- **Sensitive logging.** Secrets, JWTs, full `Authorization` headers, full request/response bodies, raw Kafka payloads, or PII in logs, traces, metric labels, or error responses are risky. Error responses from the UI API (port 5004) shouldn't leak Python stack traces.
34
+
- **Weak crypto.** MD5 or SHA-1 used for security, ECB mode, reused IVs, `random.random()` / `random.choice()` for tokens or IDs (prefer `secrets` in Python, `OsRng` / `getrandom` in Rust), or hand-rolled crypto are worth questioning. JWTs should reject the `none` algorithm, use strong secrets, and have short access-token expirations.
35
+
- **Unsafe deserialization or evaluation.** `eval`, `exec`, `pickle.loads`, `yaml.load` without `SafeLoader`, and `marshal.loads` on untrusted input are risky. Same for `eval` / the `Function` constructor / `setTimeout` with string arguments in TypeScript.
36
+
- **Removing security controls.** If a diff disables authentication, authorization, CSRF, CORS, rate limiting, or the default `127.0.0.1` bindings in `docker-compose.yaml` (see `docs/DEVELOPMENT.md` §6), ask whether it's intentional and justified.
37
+
- **Untrusted protobuf toolchain.** Per `AGENTS.md` § Security, only regenerate bindings via `./gen-protos.sh`. Flag generated-file diffs that don't match a corresponding `.proto` change.
38
+
39
+
Path-specific concerns (UI API views, output sinks, plugin adaptor, Rust coordinator, raw `.proto` changes, dependency manifests, restricted infra) are scoped in [`.coderabbit.yaml`](../.coderabbit.yaml) under `reviews.path_instructions`. When reviewing those areas, apply the same general principles — authorization checks on UI-API-supplied IDs, parameterized queries, XSS care on the client, panic-safety in Rust, license/CVE attention on dependency bumps.
40
+
41
+
## Code quality (cross-cutting)
42
+
43
+
Use judgment — these patterns tend to cause bugs or maintenance pain regardless of where they appear:
44
+
45
+
- **Generated files.** `*_pb2*.py`, `*_pb2*.pyi`, and `osprey_coordinator/src/proto/` are produced by `./gen-protos.sh`; hand-edits drift from the `.proto` schema.
46
+
- **Error handling.** Silently swallowed exceptions (`except Exception: pass` with no log or rethrow), Rust `let _ = result;` discarding an error, unhandled promise rejections in TS, and missing `await` on a coroutine whose result matters tend to cause production surprises.
47
+
- **Async correctness (Python).** Calling sync I/O inside an `async def` blocks the event loop. `asyncio.gather(...)` without `return_exceptions=True` or explicit handling can mask failures.
48
+
- **Async correctness (Rust).** Holding a `std::sync::Mutex` guard across `.await` deadlocks the tokio runtime; long-lived `tokio::sync` guards can starve other tasks.
49
+
- **Type safety.** New `# type: ignore` / `Any` / `cast(...)` in Python, `any` / `as unknown as` / non-null `!` / `@ts-ignore` in TypeScript, or `unwrap()` / `expect()` / `unsafe` on user-driven paths in Rust are worth questioning. Narrowly-scoped `# type: ignore[<code>]` and `@ts-expect-error` with a justifying comment are preferred when an escape hatch is genuinely needed (matches `AGENTS.md` § Security).
50
+
- **Tests.** New behavior generally warrants a test; bug fixes generally warrant a regression test (`AGENTS.md` § "Code review"). Integration tests run via `./run-tests.sh`.
51
+
- **Duplicated logic.** If a helper already exists in the same package, prefer it over a parallel implementation.
52
+
- **Dead code.** Commented-out blocks and TODOs without an issue link are worth a nudge.
53
+
54
+
## What not to flag
55
+
56
+
These categories of comments tend to add noise without surfacing real risk — please skip them:
57
+
58
+
- "Consider adding a `None` check" on a value already typed as non-`None` by mypy or non-`null`/`undefined` by TypeScript.
59
+
- "Consider adding error handling" on a wrapper that already propagates errors via `async`/`await` or `?` in Rust.
60
+
- "This could be a constant" on a string literal used in a single place.
61
+
- "Add a docstring" on an internal helper.
62
+
- Rhetorical questions like "have you considered…" without a concrete risk attached.
63
+
- Defensive-coding suggestions on values whose types already prevent the failure mode.
64
+
- "Add a test" on a config-only, doc-only, or comment-only change.
65
+
- Suggestions to rename a symbol "for clarity" without a concrete ambiguity.
66
+
- Style nits in generated protobuf files (`*_pb2*.py`, `*_pb2*.pyi`, `osprey_coordinator/src/proto/`) — they're excluded from `ruff` and `mypy` for a reason.
67
+
68
+
## Tone
69
+
70
+
Be specific and concise. When you flag something, name the concrete risk ("possible SQL injection via string-built `text()`", "missing authorization check — possible IDOR", "secret logged at info level", "`unwrap()` on RPC input — panics will crash the worker") and, where helpful, show the fix in code. Skip nits, and stay quiet when nothing in this list applies.
+6
.github/dependabot.yml
+6
.github/dependabot.yml
···
4
4
directory: "/"
5
5
schedule:
6
6
interval: "weekly"
7
+
labels:
8
+
- "dependencies"
9
+
- "dependabot"
7
10
ignore:
8
11
- dependency-name: "grpcio"
9
12
···
11
14
directory: "/"
12
15
schedule:
13
16
interval: "weekly"
17
+
labels:
18
+
- "dependencies"
19
+
- "dependabot"
14
20
groups:
15
21
github-actions:
16
22
patterns: