Release v0.3.0

README.md

@@ -74,7 +74,7 @@ Options (most to least common):
 | --- | --- |
 | `--pr <n>` | Review GitHub PR #n by number (diff fetched via `gh`, no checkout); not combinable with `--base`/`--head`/`--staged`. |
 | `--post` | With `--pr`, also post the result as the PR comment (needs `gh` auth). Omit to preview only; re-run with `--post` to publish. |
-| `--staged` | Review only staged changes (index vs HEAD; not combinable with `--base`/`--head`). |
+| `--staged` | Review only staged changes. |
 | `--base <ref>` | Base ref to diff against (default: merge-base with the default branch). |
 | `--head <ref>` | Head ref to diff (default: working tree, incl. uncommitted changes). |
 | `--agents <a,b>` | Run only these agents (comma-separated ids); default: all. |
@@ -176,9 +176,6 @@ per-repo `noise.additionalIgnores`.
   "policy": { "includeSuggestions": false },  // suppress suggestion-severity findings
   "chunk": { "maxChangedLines": 1000, "maxFiles": 20, "concurrency": 6 },
   "noise": { "additionalIgnores": ["packages/*/build/**"] },
-  "review": { "trigger": "all",               // which PRs `ecr ci` reviews: "all"
-              "label": "ai-review",            // (default, except ai-review:skip) or
-              "skipLabel": "ai-review:skip" }, // "label" (only labeled PRs)
   "breakGlass": { "marker": "/skip-review" }, // PR body marker that skips the review
   "commentTag": "expo-ai-code-reviewer",      // hidden tag used to find/update the comment
   "auth": { "mode": "oauth", "provider": "anthropic",
@@ -226,14 +223,6 @@ change which model reviewed your code. Use an explicit override instead.
   un-reducible pass reports a coverage gap — and it is always reported, never silent.
 - **Parse failures are retried** (same session, then once in a bounded fresh
   session) — separate from the timeout path.
-- **Transient API errors are retried** (bounded backoff on 429/5xx/network) —
-  distinct from both the timeout path (abandon) and the parse path; a one-off blip
-  no longer drops an entire pass.
-- **Auth failures surface once, and fail fast** — `ecr` checks the configured
-  provider's credential at startup and stops with one clear message if it's missing
-  (rather than failing every pass); a credential rejected mid-run (401/403)
-  collapses into a single actionable coverage note pointing at
-  `auth.tokenEnv`/`REVIEWER_MODEL`.
 - **A failed run never reads as "Approve"** — all passes fail → "could not
   complete"; some fail → never a clean approve, and coverage-reduced.
 - **The coordinator can't sink the run** — if consolidation fails, findings are
@@ -254,12 +243,9 @@ line: **comments = one-shot actions, labels = persistent configuration.**
 - **command workflow** — one-shot `/review` comments (maintainers): `/review`
   (router picks agents), `/review all`, `/review correctness security`. Never
   changes configuration.
-- **auto workflow** — continuous review. **Which PRs get reviewed is set in
-  `config.jsonc` → `review.trigger`**: `"all"` (default) reviews every PR except
-  those labeled `ai-review:skip`; `"label"` reviews only PRs labeled `ai-review`
-  (or `ai-review:<agent>` to scope agents). `ecr ci` self-gates on this policy;
-  the workflow's `if:` is an optional coarse gate layered on top. `ai-review:skip`
-  always wins and is write-gated, so a contributor can't opt their own PR out.
+- **auto workflow** — continuous review, configured by **labels**: `ai-review`
+  (router), `ai-review:all`, `ai-review:<agent>` (e.g. `ai-review:security`;
+  combine to widen), `ai-review:skip` (opt-out).
 - **dismiss workflow** — `/dismiss <id> [… -- reason]` / `/undismiss <id>`
   (maintainers). Each finding shows a short `` `id:…` ``. Dismissal is a **display
   filter only** — the reviewer still analyzes everything, and a `critical`/`secrets`
@@ -277,9 +263,7 @@ as the published package via `npx`, so no PR-controlled code is built.
 Each run appends a JSON line to `.expo-code-review/.runs/reviews.jsonl` with the
 inputs, decision, finding count, duration, per-agent cost, and aggregate token
 usage (incl. prompt-cache read/write counts) — for auditing and measuring
-cost/latency/cache reuse over time. The same totals are printed as a one-line
-summary to the terminal / CI job log at the end of each run, so cache reuse is
-visible even in CI (where the run log is ephemeral).
+cost/latency/cache reuse over time.
 
 </details>
 

ROADMAP.md

@@ -17,10 +17,10 @@ PRs** (do these first, in this order):
    re-pushes; the last remaining lever for the monster-PR case. See §5.
 2. **Cross-cutting pass parallelization / bounding** — it's the serial long-pole on
    large PRs (one pass over every changed file). Chunk it by package/proximity.
-3. ✅ **Transient-error retry with backoff** *(shipped 2026-07-23, #3.)* A non-timeout,
-   non-parse API error (one-off 429/5xx/network) used to drop that pass with no retry.
-   Now retried with bounded backoff via `isTransientApiError` (excludes
-   `AgentTimeoutError`), distinct from the timeout→subdivide and parse→retry paths.
+3. **Transient-error retry with backoff** — a non-timeout, non-parse API error
+   (one-off 429/5xx/network) currently drops that pass with no retry. It's distinct
+   from the timeout→subdivide and parse→retry paths and deserves its own bounded
+   retry. Real reliability hole, small fix.
 4. **Close the config/code injection vector (security)** — the auto-workflow builds
    + loads config from the PR merge ref, so a same-repo PR can rewrite the reviewer's
    prompts/config/code and it runs with secrets. Fix: build + load config from the
@@ -31,32 +31,6 @@ PRs** (do these first, in this order):
 
 ## Recently shipped
 
-- **Follow-up batch (2026-07-23, PRs #2–#7)** — first round of post-extraction
-  low-hanging-fruit hardening:
-  - **Transient-error retry with backoff (#3)** — a one-off 429/5xx/network error on
-    a pass is now retried with bounded backoff (2s→8s) instead of dropping the whole
-    pass; classified by `isTransientApiError`, which excludes `AgentTimeoutError`
-    (timeouts still abandon). Closes reliability/merge-boundary follow-up #3 below.
-  - **Fail-fast provider-auth check (#4)** — `checkProviderAuth` (shared by
-    `prepareAuth` + `doctor`) fails fast with one clear message before the run when
-    the configured provider has no credential, instead of surfacing as N failed
-    passes mid-run. Closes the "fail-fast provider-auth check" item under Model
-    selection below.
-  - **Auth failures collapse to one note (#5)** — a rejected credential (401/403)
-    hitting every pass now reports a single actionable coverage note (`isAuthError`)
-    instead of N generic failures. Closes the UX "auth-shaped errors → one message" item.
-  - **`--staged` + `--base`/`--head` rejected (#2)** — was silently ignored; now a
-    clear error. Closes the UX-minor `--staged`/`--base` warning item.
-  - **Usage totals surfaced to CI/terminal (#6)** — token/cache/cost totals are
-    printed as a one-line summary via progress, so prompt-cache reuse is visible in
-    CI (where `.runs/reviews.jsonl` is ephemeral). Closes the §4.1 follow-up.
-  - **Config-driven trigger policy + skip fix (#7)** — new `review.trigger`
-    (`"all"` default | `"label"`) in `config.jsonc`; `ecr ci` self-gates via
-    `shouldReview()`, honoring the write-gated `ai-review:skip` label. Fixed the
-    workflow `if:` gate: the old `contains(join(labels), 'ai-review')` matched
-    `ai-review:skip` as a substring (skip didn't skip) — now an exact array-form
-    match, as an optional coarse opt-out on top of the config policy. Template +
-    workflow kept in sync.
 - **Never-drop-work on timeout (2026-07-22)** — a hard-timed-out chunk is now
   subdivided (halved recursively down to a single file) and re-reviewed instead of
   dropped; a single file that still won't converge gets a fast no-tools fallback
@@ -288,19 +262,25 @@ What's left is at our layer, in suggested order:
      in CI, so the Anthropic (default) cache numbers aren't visible after a run —
      surface the token/cache totals into the CI job log or the run summary so CI
      caching can be confirmed the same way (Anthropic reports write + read).
-     **✅ Shipped (2026-07-23, #6):** `formatUsageSummary` prints a one-line totals
-     summary (input/output/reasoning + cache read/write + cost) via progress, so it
-     lands in the CI job log and the local terminal.
-2. ✅ **Retry in the same session, not a fresh one.** *(Already implemented.)*
-   `promptAndParse` sends the `CORRECTIVE` nudge as a follow-up in the first
-   attempt's session (the model still holds the file context — a cache read and a
-   cheap re-emit), and only falls back to a fresh session as a last resort. The
-   documented implementation trap is handled: it snapshots the message count
-   (`baseline = (await fetchMessages(...)).length`) and waits for a *new* assistant
-   message via `pollForCompletion(..., { fromIndex: baseline })`, so the
-   already-completed first message isn't returned as stale text. Timeouts still stay
-   fresh/abandon. *(Historical note: this section predates that implementation; kept
-   for the rationale.)*
+2. **Retry in the same session, not a fresh one.** `promptAndParse` retries a
+   JSON-parse failure by replaying `text + CORRECTIVE` in a **new session**,
+   discarding the first attempt's entire tool-call context (all the patch reads).
+   Sending the corrective as a follow-up message in the same session makes the
+   prior conversation a cache read and the model already holds the file context —
+   cheaper, and better for recall: today's fresh-session retry re-investigates
+   from scratch and may do so less thoroughly, whereas the common failure
+   (truncated/malformed JSON after a sound investigation) just needs a clean
+   re-emit. Shape: same-session for the first corrective retry, fresh session as
+   the last resort (clean slate for a genuinely confused attempt). Timeouts stay
+   fresh-session (we abort the stalled one — and see reliability item: don't
+   retry timeouts at all).
+   - *Implementation trap:* `promptAgent`'s poll loop returns "the last assistant
+     message with a completed timestamp" — after a follow-up prompt, the first
+     attempt's message is already completed and would be returned instantly as
+     stale text. The retry path must snapshot existing message IDs and wait for a
+     *new* assistant message.
+   - *Verify:* whether re-sending the `system` param on a follow-up prompt
+     appends a duplicate system message (perturbing the cached prefix).
 3. **Interleave tasks chunk-major, not agent-major (contingent on #1).** A cache
    entry is readable only after the first response starts streaming, so N
    parallel same-prefix requests all pay full write price. Tasks are currently
@@ -435,12 +415,10 @@ and §3. What remains, by tier:
   confirming the file exists on the base (one extra lookup per out-of-diff file),
   else plain text. Low priority — verified findings rarely cite bogus paths, and a
   404 is more visible than the dead diff anchor this replaced.*
-- ✅ **Auth-shaped (401/403) agent errors → one actionable message** *(shipped
-  2026-07-23, #5)* via `isAuthError` — collapses to a single coverage note.
+- **Auth-shaped (401/403) agent errors → one actionable message** — still open.
 - **Minor**: README uses `ecr` though unpublished (note once that real invocation
   is `yarn workspace expo-code-review dev …`); `init` next-steps vs scaffolded
-  `auth.mode`; ~~warn when `--staged` is combined with `--base`/`--head`~~ ✅ shipped
-  (#2 — now rejected with a clear error).
+  `auth.mode`; warn when `--staged` is combined with `--base`/`--head`.
 
 ## Extraction — done ✅ (remaining cleanup)
 
@@ -473,11 +451,11 @@ tool, a clear failure beats an invisible substitution. Explicit overrides
 (`REVIEWER_MODEL`, frontmatter `model:`) are the portability primitive.
 
 Wanted instead:
-- ✅ **Fail-fast provider-auth check** *(shipped 2026-07-23, #4.)* `checkProviderAuth`
-  (shared by `doctor` + `prepareAuth`) says up front when the configured provider has
-  no credential ("configured `anthropic/…` but token env X isn't set — set
-  `REVIEWER_MODEL` or authenticate") instead of surfacing as N failed passes mid-run.
-  Removes most of the perceived need for a fallback.
+- **Fail-fast provider-auth check** (in `doctor` and at run start): if the
+  configured model's provider isn't authenticated, say so up front ("configured
+  `anthropic/…` but only OpenAI is logged in — set `REVIEWER_MODEL` or
+  authenticate Anthropic") instead of surfacing as N failed passes mid-run. This
+  removes most of the perceived need for a fallback. Small, clearly good.
 - **Optional, opt-in `fallbackModel`** for availability only: fires ONLY on the
   primary being unavailable / rate-limited / errored, and is **surfaced in the
   review output** ("primary X unavailable; this pass ran on fallback Y") — never

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@expo/code-review-cli",
-  "version": "0.2.3",
+  "version": "0.3.0",
   "description": "Generic, config-driven AI code reviewer engine. Repos supply their agents via .expo-code-review/.",
   "license": "MIT",
   "repository": {