---
name: review-pr-local
specializes: review-pr
specializes_source: warpdotdev/common-skills:.agents/skills/review-pr
description: Repo-specific review guidance for warp. Only the categories declared overridable by the core review-pr skill may be specialized here.
---

# Repo-specific review guidance for `warp`

## Prerequisite: install the parent skill

This skill specializes the core `review-pr` skill (named in the `specializes` frontmatter field) and is not functional on its own. Before applying its guidance, confirm the parent skill is installed and resolvable at `.agents/skills/review-pr/SKILL.md`. If it is missing, install it first by copying the skill directory from the source declared in the `specializes_source` frontmatter field (`warpdotdev/common-skills:.agents/skills/review-pr`). Then continue with the guidance below.

This file is a companion to the core `review-pr` skill. It does not
redefine the review output schema, severity labels, safety rules, or
evidence rules. It only specializes the override categories the core
skill marks as overridable.

## Repo-specific style and recurring review patterns

- Check every PR against the testing guidelines in `.agents/skills/rust-unit-tests/SKILL.md` and `.agents/skills/gui-integration-test/SKILL.md`: flag tests the PR adds that those skills would call out, and new code that should have a test and doesn't.
- When a PR is clearly a V0 or initial implementation, frame robustness suggestions such as timeouts, retries, and lifecycle management as optional future work rather than blocking concerns, unless they risk correctness, security, data loss, or a persistent UI hang.
- For Rust changes, apply the repository conventions from `AGENTS.md`: avoid unnecessary type annotations, prefer imports over long path qualifiers, name context parameters `ctx` and place them last, remove unused parameters instead of prefixing them with `_`, and prefer inline format arguments in macros.
- When a PR adds or changes calls to log macros (`log::*` / `safe_*`) or error reporting (`report_error!` / `report_if_error!`), review it against `.agents/skills/logging-and-error-reporting/SKILL.md`. Common issues to flag: using `log::error!` for a failure that should be a Sentry issue (only `report_error!` and panics create issues — `log::*` at Error/Warn/Info are just breadcrumbs); interpolating per-instance data into a `report_error!` grouping message instead of `.context()`/`extra:`; demoting a real typed error into `extra:` or stringifying it with `anyhow!("{e}")` instead of reporting it as the payload; an inappropriate log level for hot paths; and secrets/PII in reports or Info-and-above logs (use the `safe_*` macros for sensitive detail).
- Avoid wildcard `_` match arms when an enum can reasonably be matched exhaustively; exhaustive matches are preferred so future variants are surfaced during review.
- For new or changed feature flags, prefer high-level runtime checks with `FeatureFlag::YourFlag.is_enabled()` over `#[cfg(...)]` unless the code cannot compile without a compile-time gate.
- Flag nested or redundant `TerminalModel` locking when the call stack may already hold the model lock. Prefer passing locked references down the stack and keeping lock scopes short.
- In WarpUI code, flag inline `MouseStateHandle::default()` usage during render or event handling. Mouse state handles should be created during construction and then cloned/referenced where needed.
- For user-facing UI changes, mention missing validation only when it is tied to a concrete risk or when the PR changes behavior that should be verified visually.

## Behavioral or UI-impacting changes require visual evidence

- If the PR changes anything user-visible (UI components, layout, styling, copy in surfaces users see, terminal/Warp app visuals, or other behavior a user can perceive), analyze both `pr_description.txt` and any PR comments available in the workflow context for attached screenshots, GIFs, or videos demonstrating the change end to end.
  - Treat markdown image/video embeds (`![...](...)`, `<img ...>`, `<video ...>`), GitHub user-attachment links (e.g. `https://github.com/user-attachments/...`, `https://user-images.githubusercontent.com/...`), Loom links, and similar hosted media as valid evidence.
  - The `Screenshots / Videos` section from `.github/pull_request_template.md` being present but empty does not count as evidence.
  - Unit tests, integration tests, `git diff --check`, code-path descriptions, and other textual explanations may supplement visual evidence but do not replace it for user-visible behavior.
- If the change is behavioral or UI-impacting and no screenshots or videos are attached in the description or comments, add an inline or summary-level comment requesting them. Use wording such as: "For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end."
- When required visual evidence is missing for a behavioral or UI-impacting change that can be manually tested, set the final recommendation in the top-level `body` `## Verdict` section to `Request changes`, even if no other blocking issues were found. The top-level `verdict` field must be `"REJECT"` to match.
- Author environment limitations (e.g., headless runner, no desktop, environment can't capture) do not exempt UI-impacting changes from visual evidence. Suggest capturing the recording from a local desktop run or from a remote environment with desktop/computer-use support (for example, a coding agent such as Oz with [computer use](https://docs.warp.dev/agent-platform/warps-agent/capabilities-overview/computer-use) enabled). Reply with something like: _"This change is user-facing, so a screenshot or short recording is still required. If a local desktop isn't available, you can capture it from a coding agent that supports computer use (Oz is one option — see [Warp's computer use docs](https://docs.warp.dev/agent-platform/warps-agent/capabilities-overview/computer-use)) and attach it here."_ Set the verdict to `Request changes`.
- Exempt visual evidence only when the user-visible behavior truly cannot be meaningfully shown visually (for example, changes affecting only screen readers or non-visual side effects). If so, briefly state why screenshots or recordings would not be meaningful. Never exempt based on limitations of the author's environment.
- TUI caveat: for changes to the headless TUI (`crates/warp_tui` or the cell-grid element library at `crates/warpui_core/src/elements/tui`), acceptable "visual evidence" is a terminal transcript, a `render_to_lines` / `TuiBuffer::to_lines` snapshot diff, or a `./script/run-tui` capture — NOT a `computer_use` screenshot or real-display recording (those are for the GUI desktop app). See the `tui-verify-change` skill. The `MouseStateHandle` ownership rule still applies to TUI code: the TUI's hover/click elements (`TuiHoverable`, `tui_collapsible`) are built on the shared `MouseStateHandle` and must own the handle outside render (created once, reused) so hover/click state survives rebuilt element trees — so flag inline `MouseStateHandle::default()` there too. Only the GUI's *pixel-based hit-testing* specifics are GUI-only.
- If the PR is not user-visible at all (e.g. pure refactor, internal tools, build scripts, backend-only code, tests, or documentation), do not request screenshots or videos.

## User-facing strings

- Flag interpolated text that would read unnaturally at runtime or combine sentence fragments with the wrong casing.
- Link text should be descriptive rather than bare URLs or generic "click here" labels.
- Verify that product terminology is consistent across related UI, comments, workflow messages, and errors in the same PR.

## Graceful degradation and observability

- When optional dynamic data such as URLs, session links, workflow links, issue numbers, or metadata may be absent, prefer omitting the element or showing a short fallback over rendering empty or broken output.
- Do not suggest removing session links, workflow URLs, or diagnostic context from error paths. Those links are important for debugging failed automation and user reports.
- Prefer generic, user-safe error text in user-visible surfaces, but keep enough structured logging or diagnostic context for maintainers to investigate failures.