spec: built-in /review slash command (#9606)#10014
spec: built-in /review slash command (#9606)#10014lonexreb wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
Adds product.md and tech.md for issue warpdotdev#9606: a built-in /review slash command in Agent input that gathers uncommitted local changes and asks the agent for a focused code review. V1 scope: - New StaticCommand entry alongside /init, /index, /open-code-review - Diff gathered via `git diff HEAD --no-color` (tracked) + synthesized /dev/null diffs for untracked files (respecting .gitignore via `git ls-files --others --exclude-standard`) - Truncation cap (default 50KB, configurable 1KB-1MB) with shortest-files-first preservation when over cap - Fixed V1 review prompt template prioritizing logic bugs, security, performance, simplicity, maintainability per the issue's stated focus - Availability gate: REPOSITORY | AI_ENABLED | HAS_UNCOMMITTED_CHANGES (new flag, ridding the existing file-tree-change cache) Out of V1 (tracked as follow-ups): - Commit-range review (`/review HEAD~3..HEAD`) - User-customizable prompt template - Per-file scope (`/review src/auth/`) - Streaming summary→deep-dive - Diff redaction layer for sensitive content - "Re-review" affordance after follow-ups 10 testable behavior invariants, each mapped to a test layer in tech.md. Implementation surface is small: one StaticCommand const, one dispatch arm, one new diff-gathering helper module, one settings entry, one new Availability flag. Reuses existing agent turn submission, telemetry, and Availability cache infrastructure.
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This spec proposes a built-in /review slash command that gathers local uncommitted Git changes and submits them to the Agent for focused review. The product and tech specs cover the user flow, prompt template, diff gathering, settings, risks, and test plan.
Concerns
- The spec has internal contradictions around untracked-only changes and where truncation notices appear in the fixed prompt.
- The tech plan does not match the current slash-command dispatch and availability model, so an implementation following it would either fail to wire correctly or fail the promised disabled-state UX.
Security
- The V1 design intentionally sends full diff contents, including untracked file contents, to the agent provider while deferring redaction; the spec should define a concrete V1 guardrail or user-visible disclosure before implementation.
Verdict
Found: 0 critical, 5 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| ## Prompt template (fixed in V1) | ||
|
|
||
| The agent prompt assembled by `/review` is, byte-for-byte: |
There was a problem hiding this comment.
| 4. When the gathered diff exceeds `ai.review_command_max_diff_bytes`, the prompt's diff content is truncated (longest files first), and the prompt body contains the literal substring `"This diff was truncated"`. | ||
| 5. The diff content is wrapped between the literal markers `[BEGIN DIFF]` and `[END DIFF]` exactly once each. | ||
| 6. The command's `Availability` flags include `REPOSITORY | AI_ENABLED` (matching the pattern set by `/init`). | ||
| 7. If the user has uncommitted changes only in untracked (new) files — i.e. files Git does not yet know about — the command still gathers them (via `git diff HEAD --no-color --binary` plus the file contents for new files via the existing change-detection path). |
There was a problem hiding this comment.
HAS_UNCOMMITTED_CHANGES includes untracked files and make the palette behavior match.
|
|
||
| **File:** `app/src/terminal/input/slash_commands/mod.rs`. | ||
|
|
||
| The dispatch path that consumes `SlashCommandId` already has a switch over the registry's identifiers. Add an arm: |
There was a problem hiding this comment.
command.name in execute_slash_command, and SlashCommandId is an opaque UUID rather than an enum; update the dispatch design so implementers do not add a nonexistent SlashCommandId::Review arm.
|
|
||
| **File:** `app/src/search/slash_command_menu/static_commands/mod.rs`. | ||
|
|
||
| The existing `Availability` bitflags struct gets one more flag. The condition is true when the active session's CWD is inside a Git repo *and* `git status --porcelain` returns at least one entry. Existing `Availability` flags are checked at palette-render time via cached `WorkspaceState` flags (find the cache via `grep -rn "Availability::REPOSITORY" app/src/`). The new flag rides the same cache; the cache is invalidated when the working tree changes (which the file-tree observer already tracks for the existing Code Review panel). |
There was a problem hiding this comment.
HAS_UNCOMMITTED_CHANGES as an Availability bit will hide /review when false because the data source filters inactive commands; either spec disabled-item support or change the no-changes UX to absence/toast-only.
|
|
||
| ## Risks | ||
|
|
||
| - **Diff payload as a security surface.** The diff is sent to the agent provider verbatim, including any secrets a developer accidentally committed locally (`.env` files, tokens in fixtures, etc.). **Mitigation:** the V1 prompt is silent about this; the existing AI privacy posture applies. Long-term, the existing redaction infrastructure (used for command output) could be applied to diffs — tracked as a follow-up. Document the risk in the docs page. |
There was a problem hiding this comment.
1 participant
Adds a product+tech spec for #9606: a built-in
/reviewslash command in Agent input that gathers local uncommitted changes and asks the Agent for a focused code review.Files
specs/GH9606/product.md(108 lines) — V1 scope, user experience, fixed prompt template, 10 testable behavior invariantsspecs/GH9606/tech.md(160 lines) — module layout, dispatch arm, diff-gathering helper, settings entry, end-to-end flowTotal: 383 insertions, 0 deletions. No code changes — this is a spec PR.
Why this command
Per the issue: users today get AI feedback on uncommitted changes by improvising — manual prompts ("can you review my changes?"), pasting diffs into the conversation, or building reusable skills.
/reviewmakes this discoverable as a built-in command that sits naturally next to the existing/init,/index,/fork,/open-code-review, and/pr-commentsstatic commands.V1 scope
StaticCommandentry inapp/src/search/slash_command_menu/static_commands/commands.rsalongside the existing crew.git diff HEAD --no-color(tracked, staged + unstaged) + synthesized/dev/nulldiffs for untracked files (respecting.gitignoreviagit ls-files --others --exclude-standard).ai.review_command_max_diff_bytes); over-cap diffs preserve shortest files first and the prompt notes the truncation explicitly.REPOSITORY | AI_ENABLED | HAS_UNCOMMITTED_CHANGES(new flag, riding the existing file-tree-change cache that already powers the Code Review panel's empty state).Out of V1 (tracked as follow-ups)
/review HEAD~3..HEAD).warp/review-template.md)/review src/auth/)Why this spec
ready-to-spec, no existing PR claims it.StaticCommandconst, one dispatch arm, one diff-gathering helper module, one settings entry, one newAvailabilityflag. Reuses existing agent-turn submission, telemetry, andAvailabilitycache./init(also constructs an agent prompt and submits it).Open questions for maintainers
/reviewis specced as reuse-active-or-create-new (matches the default agent prompt path), so follow-ups attach without proliferating conversations.ai.review_command_max_diff_bytes. ConfirmAISettingsis the right Rust group at implementation time.Happy to iterate on the design or tighten the V1 scope further.