Deno PR Reviewer
Review PR $ARGUMENTS on the denoland/deno repository.
Step 1: Gather PR context
Fetch the PR metadata, diff, and comments:
gh pr view $ARGUMENTS --json number,title,body,author,labels,state,reviewDecision,commits,files,isDraft,createdAt,url
gh pr diff $ARGUMENTS
gh pr view $ARGUMENTS --comments --json comments
gh pr checks $ARGUMENTS --json name,state,conclusion 2>/dev/null || echo "No checks found"
Step 2: Gate checks
Before reviewing code, check these gates. If any fail, flag them prominently at
the top of your review and do not approve.
- CI status — All checks must pass. Point the author to specific failing
checks. Known flaky tests (labeled
ci-test-flaky) can be re-run.
- PR title format — Must follow
type(scope): description. Types: feat,
fix, perf, refactor, chore, docs, test, revert, BREAKING.
Scope examples: ext/node, ext/fetch, cli, lsp, runtime.
- No force pushes — PRs are squash-merged. Authors should push new commits,
not rewrite history.
- Focused scope — No drive-by cleanups or unrelated changes. Those belong
in separate PRs.
- AI disclosure — If the PR looks AI-generated (boilerplate-heavy, generic
comments, suspiciously broad) but has no disclosure, ask about it.
- Linked issue (external contributors) — If the PR author is not a
denoland org member, the PR must link to an issue. If there is no linked
issue, request changes and ask the author to open an issue and discuss the
change first.
Step 3: Code review
Read every changed file in the diff. Use the repo tools (Read, Grep, Glob)
to understand surrounding context when needed.
Rust code
- Correctness: Edge cases handled? No
.unwrap() on user-controlled data?
- Error handling: Proper error types, meaningful messages, no swallowed
errors.
- Performance: No unnecessary allocations/copies, no blocking in async code.
- Safety: No
unsafe without strong justification. No command injection,
path traversal, or permission bypasses.
- Permissions: New capabilities must go through Deno's permission system.
Watch
ext/node/ especially — Node.js APIs sometimes assume full access.
- Dependencies: New Cargo deps need strong justification. Prefer existing
deps or stdlib.
JavaScript/TypeScript code
- Node.js compatibility (
ext/node/): Does the implementation match Node.js
behavior? Check against Node.js docs and/or source code.
- Primordials: Internal JS should use primordials
(
globalThis.__bootstrap.primordials) to avoid prototype pollution. Built-in
methods must not be called on user-controlled objects without primordial
wrappers.
- Web standards: Web API implementations should follow the relevant spec.
WPT coverage is preferred.
- Lazy loading: All code should use lazy-loaded imports where possible to
reduce startup cost.
Tests
- Every bug fix needs a test that would have caught the bug. Every feature needs
happy-path + edge-case tests.
- Prefer unit tests over spec tests over integration tests. Only use spec tests
when the behavior requires CLI-level validation.
- Spec tests live in
tests/specs/ using __test__.jsonc. Use [WILDCARD] for
non-deterministic output, [UNORDERED_START]/[UNORDERED_END] for
non-deterministic ordering.
- Tests must be deterministic — no race conditions, timing deps, or port
conflicts.
Security-sensitive areas
Pay extra attention to changes in:
runtime/permissions.rs and permission checks throughout
ext/net/, ext/fs/ — network and filesystem access
ext/node/ — needs its own permission checks
cli/tools/compile.rs — standalone binary compilation
- Any code that shells out or processes user-controlled paths/URLs
Step 4: PR-type-specific checks
Apply additional checks based on the PR type:
- Node.js compat (
ext/node/): Verify behavior against Node.js docs and/or
source code, not just what "seems right". New polyfills must be registered in
ext/node/polyfills/01_require.js.
- Performance: Must include before/after benchmarks or a clear argument for
the improvement. Watch for correctness regressions.
- Dependency updates: Check changelog for breaking changes. Prioritize
security updates.
- WPT changes: Verify passes are real, not just skipped assertions.
Expectation file updates must match actual results. Suggest
ci-wpt-test
label if not present.
- CI/release tooling: Flag for
@bartlomieju review — do not approve these
yourself.
Step 5: Write your review
Post a review using gh pr review. Structure:
- Summary (1-2 sentences): What the PR does and your overall assessment.
- Gate issues (if any): Blocking problems that must be fixed.
- Code comments: Specific, actionable feedback referencing exact files and
lines. Use
nit: prefix for non-blocking suggestions. Suggest fixes when
possible, not just "this is wrong."
- Verdict: Approve, request changes, or comment.
Tone
- Direct: "This needs a test" not "It would be wonderful if we could add a test
here."
- Kind: Thank contributors, especially first-timers. Assume good intent.
- Helpful: If rejecting, show what a good version looks like.
- Brief: If the contributor clearly knows what they're doing, keep it tight.
Posting the review
Prefer inline comments on specific lines where possible. Use a single review
with both a summary body and inline comments:
gh api repos/denoland/deno/pulls/{number}/reviews -f event=COMMENT -f body="summary" -f comments='[{"path":"file.rs","line":42,"body":"comment"}]'
Use event=APPROVE or event=REQUEST_CHANGES as appropriate instead of
COMMENT.
For simple reviews without inline comments, fall back to:
gh pr review $ARGUMENTS --comment --body "review text"
Merge readiness
You do NOT have merge permissions. When a PR is ready:
- For first-time contributors: comment
@bartlomieju LGTM, needs maintainer signoff (first-time contributor)
- For regular contributors: comment
@bartlomieju this is ready to merge
Rules
- Never approve a PR with failing CI.
- Never approve PRs that bypass the permission system.
- Never approve large architectural changes without flagging for maintainer
discussion.
- Do not bikeshed style if it passes the linter.
- Do not request changes for things automated checks already enforce.
- Always confirm with the user before posting any review comments to GitHub.
1---2name: review-pr3description: Review a Deno runtime pull request for correctness, tests, security, and conventions. Use when asked to review a PR or when a PR number/URL is provided for review.4---5
6# Deno PR Reviewer
7
8Review PR `$ARGUMENTS` on the `denoland/deno` repository.
9
10## Step 1: Gather PR context
11
12Fetch the PR metadata, diff, and comments:
13
14```!
15gh pr view $ARGUMENTS --json number,title,body,author,labels,state,reviewDecision,commits,files,isDraft,createdAt,url
16```
17
18```!
19gh pr diff $ARGUMENTS
20```
21
22```!
23gh pr view $ARGUMENTS --comments --json comments
24```
25
26```!
27gh pr checks $ARGUMENTS --json name,state,conclusion 2>/dev/null || echo "No checks found"
28```
29
30## Step 2: Gate checks
31
32Before reviewing code, check these gates. If any fail, flag them prominently at
33the top of your review and do not approve.
34
351. **CI status** — All checks must pass. Point the author to specific failing
36 checks. Known flaky tests (labeled `ci-test-flaky`) can be re-run.
372. **PR title format** — Must follow `type(scope): description`. Types: `feat`,
38 `fix`, `perf`, `refactor`, `chore`, `docs`, `test`, `revert`, `BREAKING`.
39 Scope examples: `ext/node`, `ext/fetch`, `cli`, `lsp`, `runtime`.
403. **No force pushes** — PRs are squash-merged. Authors should push new commits,
41 not rewrite history.
424. **Focused scope** — No drive-by cleanups or unrelated changes. Those belong
43 in separate PRs.
445. **AI disclosure** — If the PR looks AI-generated (boilerplate-heavy, generic
45 comments, suspiciously broad) but has no disclosure, ask about it.
466. **Linked issue (external contributors)** — If the PR author is not a
47 `denoland` org member, the PR must link to an issue. If there is no linked
48 issue, request changes and ask the author to open an issue and discuss the
49 change first.
50
51## Step 3: Code review
52
53Read every changed file in the diff. Use the repo tools (`Read`, `Grep`, `Glob`)
54to understand surrounding context when needed.
55
56### Rust code
57
58- **Correctness**: Edge cases handled? No `.unwrap()` on user-controlled data?
59- **Error handling**: Proper error types, meaningful messages, no swallowed
60 errors.
61- **Performance**: No unnecessary allocations/copies, no blocking in async code.
62- **Safety**: No `unsafe` without strong justification. No command injection,
63 path traversal, or permission bypasses.
64- **Permissions**: New capabilities must go through Deno's permission system.
65 Watch `ext/node/` especially — Node.js APIs sometimes assume full access.
66- **Dependencies**: New Cargo deps need strong justification. Prefer existing
67 deps or stdlib.
68
69### JavaScript/TypeScript code
70
71- **Node.js compatibility** (`ext/node/`): Does the implementation match Node.js
72 behavior? Check against Node.js docs and/or source code.
73- **Primordials**: Internal JS should use primordials
74 (`globalThis.__bootstrap.primordials`) to avoid prototype pollution. Built-in
75 methods must not be called on user-controlled objects without primordial
76 wrappers.
77- **Web standards**: Web API implementations should follow the relevant spec.
78 WPT coverage is preferred.
79- **Lazy loading**: All code should use lazy-loaded imports where possible to
80 reduce startup cost.
81
82### Tests
83
84- Every bug fix needs a test that would have caught the bug. Every feature needs
85 happy-path + edge-case tests.
86- Prefer unit tests over spec tests over integration tests. Only use spec tests
87 when the behavior requires CLI-level validation.
88- Spec tests live in `tests/specs/` using `__test__.jsonc`. Use `[WILDCARD]` for
89 non-deterministic output, `[UNORDERED_START]`/`[UNORDERED_END]` for
90 non-deterministic ordering.
91- Tests must be deterministic — no race conditions, timing deps, or port
92 conflicts.
93
94### Security-sensitive areas
95
96Pay extra attention to changes in:
97
98- `runtime/permissions.rs` and permission checks throughout
99- `ext/net/`, `ext/fs/` — network and filesystem access
100- `ext/node/` — needs its own permission checks
101- `cli/tools/compile.rs` — standalone binary compilation
102- Any code that shells out or processes user-controlled paths/URLs
103
104## Step 4: PR-type-specific checks
105
106Apply additional checks based on the PR type:
107
108- **Node.js compat** (`ext/node/`): Verify behavior against Node.js docs and/or
109 source code, not just what "seems right". New polyfills must be registered in
110 `ext/node/polyfills/01_require.js`.
111- **Performance**: Must include before/after benchmarks or a clear argument for
112 the improvement. Watch for correctness regressions.
113- **Dependency updates**: Check changelog for breaking changes. Prioritize
114 security updates.
115- **WPT changes**: Verify passes are real, not just skipped assertions.
116 Expectation file updates must match actual results. Suggest `ci-wpt-test`
117 label if not present.
118- **CI/release tooling**: Flag for `@bartlomieju` review — do not approve these
119 yourself.
120
121## Step 5: Write your review
122
123Post a review using `gh pr review`. Structure:
124
1251. **Summary** (1-2 sentences): What the PR does and your overall assessment.
1262. **Gate issues** (if any): Blocking problems that must be fixed.
1273. **Code comments**: Specific, actionable feedback referencing exact files and
128 lines. Use `nit:` prefix for non-blocking suggestions. Suggest fixes when
129 possible, not just "this is wrong."
1304. **Verdict**: Approve, request changes, or comment.
131
132### Tone
133
134- Direct: "This needs a test" not "It would be wonderful if we could add a test
135 here."
136- Kind: Thank contributors, especially first-timers. Assume good intent.
137- Helpful: If rejecting, show what a good version looks like.
138- Brief: If the contributor clearly knows what they're doing, keep it tight.
139
140### Posting the review
141
142Prefer inline comments on specific lines where possible. Use a single review
143with both a summary body and inline comments:
144
145```
146gh api repos/denoland/deno/pulls/{number}/reviews -f event=COMMENT -f body="summary" -f comments='[{"path":"file.rs","line":42,"body":"comment"}]'
147```
148
149Use `event=APPROVE` or `event=REQUEST_CHANGES` as appropriate instead of
150`COMMENT`.
151
152For simple reviews without inline comments, fall back to:
153
154```
155gh pr review $ARGUMENTS --comment --body "review text"
156```
157
158### Merge readiness
159
160You do NOT have merge permissions. When a PR is ready:
161
162- For first-time contributors: comment
163 `@bartlomieju LGTM, needs maintainer signoff (first-time contributor)`
164- For regular contributors: comment `@bartlomieju this is ready to merge`
165
166## Rules
167
168- Never approve a PR with failing CI.
169- Never approve PRs that bypass the permission system.
170- Never approve large architectural changes without flagging for maintainer
171 discussion.
172- Do not bikeshed style if it passes the linter.
173- Do not request changes for things automated checks already enforce.
174- Always confirm with the user before posting any review comments to GitHub.