| 1 | //! The `/review` slash command (a port of opencode's built-in `/review`). |
| 2 | //! |
| 3 | //! This mirrors opencode's implementation end to end: |
| 4 | //! |
| 5 | //! - **Template**: opencode's `packages/opencode/src/command/template/review.txt`, |
| 6 | //! with `$ARGUMENTS` standing in for whatever the user typed after the |
| 7 | //! command (a commit hash, branch name, or PR URL/number; empty = review all |
| 8 | //! uncommitted changes). The only deviations from opencode's template are the |
| 9 | //! tool names in the **Tools** section, adapted to this agent's actual tools |
| 10 | //! (`task` with the `explore` sub-agent, `search`, and `web_search` — opencode |
| 11 | //! lists an "Explore agent", "Exa Code Context", and "Web Search" instead). |
| 12 | //! |
| 13 | //! - **Args**: opencode's `SessionPrompt.command` substitution rules, faithfully |
| 14 | //! reproduced in [`render_command_template`]: arguments are tokenized with |
| 15 | //! opencode's `argsRegex` (whitespace-separated, single/double quotes kept |
| 16 | //! intact and then trimmed), `$1`, `$2`, … are replaced positionally with the |
| 17 | //! *last* numbered placeholder swallowing any extra arguments, `$ARGUMENTS` is |
| 18 | //! replaced with the raw argument string, and when the template has no |
| 19 | //! placeholders the arguments are appended to the end of the template. |
| 20 | //! |
| 21 | //! - **Execution** (handled in `main.rs`): opencode defines `/review` with |
| 22 | //! `subtask: true`, so the template is never inserted into the session's user |
| 23 | //! message. It is handed to a sub-agent in a separate context; only the final |
| 24 | //! report is appended to the session as the assistant message. |
| 25 | |
| 26 | /// opencode: `argsRegex = /(?:[^\s"']+|"[^"]*"|'[^']*')+/g`, then |
| 27 | /// `quoteTrimRegex = /^["']|["']$/g` strips the surrounding quotes from each |
| 28 | /// matched token. This tokenizer reproduces that behavior: whitespace splits |
| 29 | /// tokens, and single/double quoted runs are kept whole with the quotes |
| 30 | /// stripped. |
| 31 | fn tokenize_args(arguments: |
| 32 | let mut out |
| 33 | let mut chars |
| 34 | let mut current |
| 35 | while let Some |
| 36 | match c { |
| 37 | '"' | '\' |
| 38 | // Quoted run: consume the opening quote, keep everything until |
| 39 | // the matching closing quote (or end of input), and append the |
| 40 | // contents to the token — quotes themselves are dropped, like |
| 41 | // `quoteTrimRegex`. |
| 42 | let quote = |
| 43 | chars.next(); |
| 44 | for ch in chars |
| 45 | if ch == quote |
| 46 | break; |
| 47 | } |
| 48 | current.push(ch |
| 49 | } |
| 50 | } |
| 51 | c if c. |
| 52 | if !current.is_empty |
| 53 | out.push(std |
| 54 | } |
| 55 | // Collapse runs of whitespace between tokens. |
| 56 | while chars.peek() |
| 57 | chars.next(); |
| 58 | } |
| 59 | } |
| 60 | c => { |
| 61 | current.push(c |
| 62 | chars.next(); |
| 63 | } |
| 64 | } |
| 65 | } |
| 66 | if !current.is_empty |
| 67 | out.push(current |
| 68 | } |
| 69 | out |
| 70 | } |
| 71 | |
| 72 | /// opencode's `placeholderRegex = /\$(\d+)/g` — the `N` of every `$N`. |
| 73 | fn numbered_placeholders(template: |
| 74 | let mut out |
| 75 | let bytes = |
| 76 | let mut i |
| 77 | while i < bytes |
| 78 | if bytes[i] |
| 79 | let mut j |
| 80 | let mut n |
| 81 | let mut found |
| 82 | while j < bytes |
| 83 | n = n |
| 84 | j += 1; |
| 85 | found = true; |
| 86 | } |
| 87 | if found { |
| 88 | out.push(n |
| 89 | i = j; |
| 90 | continue; |
| 91 | } |
| 92 | } |
| 93 | i += 1; |
| 94 | } |
| 95 | out |
| 96 | } |
| 97 | |
| 98 | /// Render a command template the way opencode's `SessionPrompt.command` does: |
| 99 | /// |
| 100 | /// 1. Parse the user's arguments into quote-aware tokens. |
| 101 | /// 2. Replace `$1`, `$2`, … positionally; a placeholder with no corresponding |
| 102 | /// argument becomes empty, and the **last** numbered placeholder swallows |
| 103 | /// any extra arguments joined with spaces. |
| 104 | /// 3. Replace `$ARGUMENTS` with the raw argument string. |
| 105 | /// 4. If the template has no placeholders at all but arguments were given, |
| 106 | /// append `\n\n` + the raw arguments (so free-form templates still see |
| 107 | /// the user's input). |
| 108 | pub fn render_command_template(template |
| 109 | let args = |
| 110 | let placeholders = |
| 111 | let last = |
| 112 | |
| 113 | let mut out |
| 114 | let bytes = |
| 115 | let mut i |
| 116 | while i < bytes |
| 117 | if bytes[i] |
| 118 | let mut j |
| 119 | let mut n |
| 120 | let mut found |
| 121 | while j < bytes |
| 122 | n = n |
| 123 | j += 1; |
| 124 | found = true; |
| 125 | } |
| 126 | if found { |
| 127 | let arg_index = |
| 128 | if arg_index >= args |
| 129 | // Missing argument → empty string. |
| 130 | } else if Some |
| 131 | // The final placeholder absorbs the remaining arguments. |
| 132 | out.push_str(&args |
| 133 | } else { |
| 134 | out.push_str(&args |
| 135 | } |
| 136 | i = j; |
| 137 | continue; |
| 138 | } |
| 139 | } |
| 140 | out.push(bytes |
| 141 | i += 1; |
| 142 | } |
| 143 | |
| 144 | let uses_arguments = |
| 145 | let mut rendered |
| 146 | if placeholders.is_empty() |
| 147 | rendered.push_str("\ |
| 148 | rendered.push_str(arguments |
| 149 | } |
| 150 | rendered.trim().to_string |
| 151 | } |
| 152 | |
| 153 | /// The review prompt. `arguments` is whatever followed the slash command — a |
| 154 | /// commit hash, branch name, or PR URL/number — and is substituted for the |
| 155 | /// template's `$ARGUMENTS` placeholders. An empty string means "review all |
| 156 | /// uncommitted changes". |
| 157 | pub fn review_template(arguments |
| 158 | render_command_template(TEMPLATE, arguments |
| 159 | } |
| 160 | |
| 161 | /// opencode's `command/template/review.txt`, adapted to this agent's tools. |
| 162 | const TEMPLATE: &str |
| 163 | |
| 164 | --- |
| 165 | |
| 166 | Input: $ARGUMENTS |
| 167 | |
| 168 | --- |
| 169 | |
| 170 | ## Determining What to Review |
| 171 | |
| 172 | Based on the input provided, determine which type of review to perform: |
| 173 | |
| 174 | 1. **No arguments (default)**: Review all uncommitted changes |
| 175 | - Run: `git diff` for unstaged changes |
| 176 | - Run: `git diff --cached` for staged changes |
| 177 | - Run: `git status --short` to identify untracked (net new) files |
| 178 | 2. **Commit hash** (40-char SHA or short hash): Review that specific commit |
| 179 | - Run: `git show $ARGUMENTS` |
| 180 | 3. **Branch name**: Compare current branch to the specified branch |
| 181 | - Run: `git diff $ARGUMENTS...HEAD` |
| 182 | 4. **PR URL or number** (contains "github.com" or "pull" or looks like a PR number): Review the pull request |
| 183 | - Run: `gh pr view $ARGUMENTS` to get PR context |
| 184 | - Run: `gh pr diff $ARGUMENTS` to get the diff |
| 185 | |
| 186 | Use best judgement when processing input. |
| 187 | |
| 188 | --- |
| 189 | |
| 190 | ## Gathering Context |
| 191 | |
| 192 | **Diffs alone are not enough.** After getting the diff, read the entire file(s) being modified to understand the full context. Code that looks wrong in isolation may be correct given surrounding logic—and vice versa. |
| 193 | |
| 194 | - Use the diff to identify which files changed |
| 195 | - Use `git status --short` to identify untracked files, then read their full contents |
| 196 | - Read the full file to understand existing patterns, control flow, and error handling |
| 197 | - Check for existing style guide or conventions files (CONVENTIONS.md, AGENTS.md, .editorconfig, etc.) |
| 198 | |
| 199 | --- |
| 200 | |
| 201 | ## What to Look For |
| 202 | |
| 203 | **Bugs** |
| 204 | - Your primary focus. |
| 205 | - Logic errors, off-by-one mistakes, incorrect conditionals |
| 206 | - If-else guards: missing guards, incorrect branching, unreachable code paths |
| 207 | - Edge cases: null/empty/undefined inputs, error conditions, race conditions |
| 208 | - Security issues: injection, auth bypass, data exposure |
| 209 | - Broken error handling that swallows failures, throws unexpectedly or returns error types that are not caught. |
| 210 | |
| 211 | **Structure** |
| 212 | - Does the code fit the codebase? |
| 213 | - Does it follow existing patterns and conventions? |
| 214 | - Are there established abstractions it should use but doesn't? |
| 215 | - Excessive nesting that could be flattened with early returns or extraction |
| 216 | |
| 217 | **Performance** |
| 218 | - Only flag if obviously problematic. |
| 219 | - O(n²) on unbounded data, N+1 queries, blocking I/O on hot paths |
| 220 | |
| 221 | **Behavior Changes** |
| 222 | - If a behavioral change is introduced, raise it (especially if it's possibly unintentional). |
| 223 | |
| 224 | --- |
| 225 | |
| 226 | ## Before You Flag Something |
| 227 | |
| 228 | **Be certain.** If you're going to call something a bug, you need to be confident it actually is one. |
| 229 | - Only review the changes - do not review pre-existing code that wasn't modified |
| 230 | - Don't flag something as a bug if you're unsure - investigate first |
| 231 | - Don't invent hypothetical problems - if an edge case matters, explain the realistic scenario where it breaks |
| 232 | - If you need more context to be sure, use the tools below to get it |
| 233 | |
| 234 | **Don't be a zealot about style.** When checking code against conventions: |
| 235 | - Verify the code is *actually* in violation. Don't complain about else statements if early returns are already being used correctly. |
| 236 | - Some "violations" are acceptable when they're the simplest option. A `let` statement is fine if the alternative is convoluted. |
| 237 | - Excessive nesting is a legitimate concern regardless of other style choices. |
| 238 | - Don't flag style preferences as issues unless they clearly violate established project conventions. |
| 239 | |
| 240 | --- |
| 241 | |
| 242 | ## Tools |
| 243 | |
| 244 | Use these to inform your review: |
| 245 | - **Explore sub-agent** — the `task` tool with `subagent_type: "explore"`. Find how existing code handles similar problems. Check patterns, conventions, and prior art before claiming something doesn't fit. |
| 246 | - **Search** — the `search` tool. Verify correct usage of libraries/APIs and find existing patterns before flagging something as wrong. |
| 247 | - **Web Search** — the `web_search` tool. Research best practices if you're unsure about a pattern. |
| 248 | |
| 249 | If you're uncertain about something and can't verify it with these tools, say "I'm not sure about X" rather than flagging it as a definite issue. |
| 250 | |
| 251 | --- |
| 252 | |
| 253 | ## Output |
| 254 | |
| 255 | 1. If there is a bug, be direct and clear about why it is a bug. |
| 256 | 2. Clearly communicate severity of issues. Do not overstate severity. |
| 257 | 3. Critiques should clearly and explicitly communicate the scenarios, environments, or inputs that are necessary for the bug to arise. The comment should immediately indicate that the issue's severity depends on these factors. |
| 258 | 4. Your tone should be matter-of-fact and not accusatory or overly positive. It should read as a helpful AI assistant suggestion without sounding too much like a human reviewer. |
| 259 | 5. Write so the reader can quickly understand the issue without reading too closely. |
| 260 | 6. AVOID flattery, do not give any comments that are not helpful to the reader. Avoid phrasing like "Great job ...", "Thanks for ...". |
| 261 | "#; |
| 262 | |
| 263 | #[cfg(test)] |
| 264 | mod tests { |
| 265 | use super::*; |
| 266 | |
| 267 | #[test] |
| 268 | fn default_review_targets_uncommitted_changes() { |
| 269 | let t = |
| 270 | assert!(t.starts_with |
| 271 | // Input line is empty, so the agent falls through to the default path. |
| 272 | assert!(t.contains |
| 273 | assert!(t.contains |
| 274 | assert!(t.contains |
| 275 | assert!(t.contains |
| 276 | assert!(t.contains |
| 277 | } |
| 278 | |
| 279 | #[test] |
| 280 | fn commit_hash_is_substituted_into_the_template() { |
| 281 | let t = |
| 282 | assert!(t.contains |
| 283 | assert!(t.contains |
| 284 | } |
| 285 | |
| 286 | #[test] |
| 287 | fn branch_name_is_substituted_into_the_template() { |
| 288 | let t = |
| 289 | assert!(t.contains |
| 290 | assert!(t.contains |
| 291 | } |
| 292 | |
| 293 | #[test] |
| 294 | fn pr_reference_is_substituted_into_the_template() { |
| 295 | let t = |
| 296 | assert!(t.contains |
| 297 | assert!(t.contains |
| 298 | assert!(t.contains |
| 299 | } |
| 300 | |
| 301 | #[test] |
| 302 | fn quoted_arguments_survive_into_the_template() { |
| 303 | // opencode substitutes `$ARGUMENTS` with the raw argument string, so a |
| 304 | // branch with a space keeps its quotes in the shell commands. |
| 305 | let t = |
| 306 | assert!(t.contains |
| 307 | assert!(t.contains |
| 308 | } |
| 309 | |
| 310 | #[test] |
| 311 | fn template_mentions_the_agents_tools() { |
| 312 | let t = |
| 313 | // Adapted from opencode's template: the tool names match this agent's |
| 314 | // actual tools so the reviewer can use them. |
| 315 | assert!(t.contains |
| 316 | assert!(t.contains |
| 317 | assert!(t.contains |
| 318 | assert!(t.contains |
| 319 | } |
| 320 | |
| 321 | #[test] |
| 322 | fn arguments_placeholder_passes_whitespace_through_raw() { |
| 323 | // opencode's `SessionPrompt.command` replaces `$ARGUMENTS` with the raw |
| 324 | // argument string — no trimming (the TUI's split/join round-trip keeps |
| 325 | // leading/trailing spaces as empty tokens). Callers that want clean |
| 326 | // arguments trim before rendering; `handle_line` does exactly that for |
| 327 | // `/review`, so this only matters when `review_template` is fed raw |
| 328 | // input directly. |
| 329 | let t = |
| 330 | assert!(t.contains |
| 331 | assert!(t.contains |
| 332 | } |
| 333 | |
| 334 | #[test] |
| 335 | fn tokenizer_splits_on_whitespace_and_keeps_quotes() { |
| 336 | assert_eq!(tokenize_args( |
| 337 | assert_eq!(tokenize_args( |
| 338 | assert_eq!(tokenize_args( |
| 339 | assert_eq!(tokenize_args( |
| 340 | assert_eq!(tokenize_args( |
| 341 | assert_eq!(tokenize_args( |
| 342 | // opencode's regex keeps the quotes on the token and `quoteTrimRegex` |
| 343 | // strips only the surrounding quotes — a bare quoted token ends up |
| 344 | // without quotes, exactly as here. |
| 345 | assert_eq!(tokenize_args( |
| 346 | } |
| 347 | |
| 348 | #[test] |
| 349 | fn numbered_placeholders_are_positional_and_last_absorbs() { |
| 350 | assert_eq!(render_command_template( |
| 351 | assert_eq!(render_command_template( |
| 352 | assert_eq!(render_command_template( |
| 353 | assert_eq!(render_command_template( |
| 354 | // Missing arguments become empty strings — opencode returns `""` for a |
| 355 | // placeholder with no corresponding argument (`argIndex >= args.length` |
| 356 | // in `SessionPrompt.command`), so `$1/$2` with one arg is `only/`. |
| 357 | assert_eq!(render_command_template( |
| 358 | } |
| 359 | |
| 360 | #[test] |
| 361 | fn arguments_placeholder_gets_the_raw_string() { |
| 362 | assert_eq!( |
| 363 | render_command_template("run: $ARGUMENTS", |
| 364 | "run: my feature" |
| 365 | ); |
| 366 | assert_eq!( |
| 367 | render_command_template("run: $ARGUMENTS", |
| 368 | "run: \"my feature\"" |
| 369 | ); |
| 370 | } |
| 371 | |
| 372 | #[test] |
| 373 | fn no_placeholders_appends_arguments() { |
| 374 | assert_eq!(render_command_template( |
| 375 | assert_eq!(render_command_template( |
| 376 | // Only when the template has no `$ARGUMENTS` either. |
| 377 | assert_eq!( |
| 378 | render_command_template("show $ARGUMENTS", |
| 379 | "show extra" |
| 380 | ); |
| 381 | } |
| 382 | |
| 383 | #[test] |
| 384 | fn rendered_template_is_trimmed() { |
| 385 | assert_eq!(render_command_template( |
| 386 | } |
| 387 | } |