Anatomy of a review comment

### :hourglass_flowing_sand: ADO Pilot — **review queued**

Your PR is in the review queue. This comment will update as the review progresses.

<sub>ADO Pilot · [settings](https://app.adopilot.dev/orgs/{orgId}/settings)</sub>

### :gear: ADO Pilot — **review in progress**

Analyzing 14 files (+412 / −87 lines). Expected completion: ~3 minutes.

<sub>ADO Pilot · [live progress](https://app.adopilot.dev/reviews/{reviewId})</sub>

### :white_check_mark: ADO Pilot — **PASS** · no findings

**Summary.** Small, tightly scoped change. The accompanying test fixture update is correct. No issues found.

---

<sub>ADO Pilot v{semver} · [full review](https://app.adopilot.dev/reviews/{reviewId}) · [settings](https://app.adopilot.dev/orgs/{orgId}/settings) · model: claude-sonnet-4-6 · took 48s</sub>

### :warning: ADO Pilot — **ADVISORY** · 0 blockers · 1 warning · 1 suggestion

**Summary.** The PR introduces a login feature with no blocking issues. One error-handling warning is worth fixing before merge, one refactor is optional. Style otherwise consistent with the surrounding codebase.

- :warning: **warning · error-handling** — `src/jobs/export.ts:118` logs and continues inside `catch`, masking upstream failures.
- :bulb: **suggestion · maintainability** — `src/utils/format.ts:8` is duplicated across 4 callsites; one helper would DRY these up.

---

<sub>ADO Pilot v{semver} · [full review](https://app.adopilot.dev/reviews/{reviewId}) · [settings](https://app.adopilot.dev/orgs/{orgId}/settings) · model: claude-sonnet-4-6 · took 2m 41s</sub>

### :x: ADO Pilot — **FAIL** · 2 blockers · 3 warnings · 1 suggestion

**Summary.** Two security blockers on the new auth paths: one SQL injection in the login handler, one reset-token leak in the password reset response. Both are reproducible with concrete fixes inline.

- :red_circle: **blocker · security** — `src/api/login.ts:42` concatenates `email` into a raw SQL query.
- :red_circle: **blocker · security** — `src/api/reset.ts:91` returns the reset token in the response body, leaking it past the email channel.
- :warning: **warning · error-handling** — `src/jobs/export.ts:118` swallows rejections inside `catch`.
- :warning: **warning · error-handling** — `src/api/reset.ts:54` does not handle the DB timeout case.
- :warning: **warning · testing** — `src/api/login.test.ts:12` no longer covers the happy path after the refactor.

…and 1 more — see [full review](https://app.adopilot.dev/reviews/{reviewId}).

---

<sub>ADO Pilot v{semver} · [full review](https://app.adopilot.dev/reviews/{reviewId}) · [settings](https://app.adopilot.dev/orgs/{orgId}/settings) · model: claude-sonnet-4-6 · took 3m 12s</sub>

### :warning: ADO Pilot — **review could not complete**

We hit an error partway through and couldn't finish. You will not be billed for this run. A new review will trigger automatically on the next push, or [retry manually](https://app.adopilot.dev/reviews/{reviewId}/retry).

**Error class:** `PROVIDER_RATE_LIMITED` — upstream model provider throttled us.

<sub>ADO Pilot · [diagnostics](https://app.adopilot.dev/reviews/{reviewId}) · [support](https://adopilot.dev/support)</sub>

:red_circle: **blocker · security** — SQL injection: user email concatenated into raw query

Line 42 builds the query string by concatenating the `email` variable directly into SQL text. A malicious `email` value can escape the quoting and exfiltrate or mutate other rows.

```suggestion
const user = await db.query(
  sql`SELECT * FROM users WHERE email = ${email}`,
);
```

The pieces are:

- **Headline.** Severity glyph, bold `severity · category`, and the one-liner problem statement.
- **Evidence.** Prose explaining why this is an issue, citing variable names and language semantics where it helps.
- **Suggestion block (optional).** A fenced ` ```suggestion` block. Azure DevOps renders it as a one-click "Apply change" button. ADO Pilot only emits a suggestion when the fix is short, uses imports that already exist in the file, and Pass 2 is confident the change compiles.
- **Related locations (optional).** A short note pointing at up to three other places the same pattern appears.

There is no `<sub>` footer on inline comments — the finding is the comment, and the tracking comment already carries the deep link.

## Verdict and merge

| Verdict | Glyph | Meaning | Can you merge? |
| --- | --- | --- | --- |
| PASS | green check | No issues found. | Yes. |
| ADVISORY | warning | Warnings or suggestions only, no blockers. | Yes, but read the findings first. |
| FAIL | red x | At least one blocker. | Only if `ai-pr-review` is not a required branch policy. |

For the rules that map findings to verdicts, see [Severities and categories](severities-and-categories.md).

<!-- AUTHORING-NOTE:
- Verbatim Mockup E / E' code blocks are quoted from docs/design/pr-comment-mockups.md §Mockup E. Keep these in sync if the spec template changes.
- Sources: SPEC-001 REQ-WB-003 / REQ-WB-008, high-level-design §8.3 (tracking comment lifecycle), §8.4 (PR status), pr-comment-mockups.md §Mockup E.
- Mockup E in `docs/design/pr-comment-mockups.md` line 408 contains the same inconsistency this page just fixed (its ADVISORY block lists a blocker, which by SPEC-005 REQ-010 must promote to FAIL). Design doc needs a follow-up edit to drop the `:red_circle: blocker · security` bullet and update the header count from `1 blocker · 1 warning · 1 suggestion` to `0 blockers · 1 warning · 1 suggestion`. This page intentionally diverges from the mockup until that edit lands.
- Glyphs in body prose are described in words ("green check", "warning sign") to comply with the no-emoji-in-prose standard. The fenced code blocks keep the literal `:emoji:` shortcodes because they show what the rendered comment actually contains.
- Screenshot opportunity: live ADVISORY comment from the dev org for /img/docs/reviews/tracking-comment-advisory.png.
-->