release: v0.10.0

.cargo/audit.toml

@@ -0,0 +1,40 @@
+# cargo-audit configuration for review-engine.
+# Read automatically by the rustsec/audit-check@v2 GitHub Action
+# (.github/workflows/audit.yml) and by `cargo audit` when run from the
+# repository root.
+
+[advisories]
+# RUSTSEC-2023-0071: rsa 0.9.10 — Marvin Attack (timing side-channel on RSA
+# private-key decryption).
+#
+# Why this ignore is safe for review-engine:
+#
+# 1. The vulnerable crate is never compiled. `rsa` enters Cargo.lock only as
+#    an *optional* dependency of `sqlx-mysql`, which sqlx 0.8 locks into the
+#    lockfile regardless of feature activation. The workspace enables sqlx
+#    with `default-features = false` and features = ["any", "runtime-tokio",
+#    "postgres", "sqlite", "migrate", "macros", "chrono", "uuid", "json"] —
+#    no `mysql`. Verified: `cargo tree -i rsa` and
+#    `cargo tree --all-features --target all -i rsa` both print nothing,
+#    i.e. no feature combination of this workspace activates sqlx-mysql/rsa
+#    in the build graph, and no compilation artifact for rsa is produced.
+#    Removing it from Cargo.lock is impossible while sqlx is a dependency
+#    (confirmed by regenerating the lockfile from scratch — rsa reappears).
+#
+# 2. Even if it were compiled, the attack surface does not exist here. The
+#    Marvin attack requires the attacker to submit chosen ciphertexts to an
+#    RSA private-key decryption (PKCS#1 v1.5 unpadding) oracle. review-engine
+#    performs no RSA private-key operations of any kind: no RSA code is
+#    referenced in src/ or tests/ (grep-verified), and the only consumer in
+#    the tree would be sqlx-mysql's MySQL `caching_sha2_password` auth —
+#    a code path that is unreachable because no MySQL backend is configured
+#    or supported (SQLite/PostgreSQL only; no mysql connection string is
+#    accepted anywhere).
+#
+# 3. No fix is available upstream: RUSTSEC-2023-0071 has no patched release
+#    in the rsa 0.9 series; upgrading within sqlx's accepted range cannot
+#    resolve it. Revisit this ignore if a fixed rsa release lands or if the
+#    project ever enables sqlx's `mysql` feature.
+ignore = [
+    "RUSTSEC-2023-0071",
+]

CHANGELOG.md

@@ -1,5 +1,38 @@
 # Changelog
 
+## [0.10.0] - 2026-09-04
+
+> **0.10.x is a major upgrade.** The upgrade itself is fully automatic — the first 0.10.x boot creates the database, applies the embedded schema migrations, and imports `ui-state.toml` into the database in one transaction — but it is a **one-way move: downgrading back to 0.9.x is not supported** (configuration's authoritative source becomes the database and `ui-state.toml` is renamed to `ui-state.toml.migrated`, which a 0.9.x binary cannot read). **Back up your config directory** (`~/.config/review-engine/`, including `ui-state.toml` and `secrets.key`; Docker: the `./config` and `./auth` bind mounts) **before upgrading.** Step-by-step instructions, expected logs, verification, and troubleshooting: [`docs/migration-0.10.md`](docs/migration-0.10.md).
+
+### Added
+- **Persistent storage layer — PostgreSQL primary, embedded SQLite fallback**: a new `src/store/` module on sqlx 0.8's `Any` pool serves both backends from one code path; `DATABASE_URL` set → PostgreSQL, unset → embedded SQLite at `~/.config/review-engine/review.db`. The initial migration creates 7 tables (`reviews` / `expert_reports` / `mr_discussions` / `review_contexts` / `git_platforms` / `llm_providers` / `app_settings`), embedded via `sqlx::migrate!()` and applied at startup. Dialect rules (placeholders, JSON-as-TEXT, bool-as-INTEGER, RFC 3339 TEXT timestamps) are codified in `design/persistence.md` §3.1. (`src/store/`, `migrations/0001_init.sql`, `Cargo.toml`)
+- **Review history survives restarts**: `TaskStore` now writes through to the DB, and with persistence active the History list/detail APIs read from the DB — history is no longer bounded by the 30-minute in-memory reaper window. A startup sweep flips rows still `pending`/`running` when the previous process died to `failed` with `error='interrupted: server restarted'`, so stale tasks never hang in a running state. `db=None` keeps the exact 0.9 in-memory path. (`src/server/task_queue.rs`, `src/server/api/review/handlers.rs`, `src/store/traits.rs`, `src/store/sqlx.rs`)
+- **Configuration in the database**: git platforms and LLM provider configs move from `ui-state.toml` into the DB via a one-shot, single-transaction import on first boot (a mid-import failure rolls back cleanly and retries next startup); the file is renamed to `ui-state.toml.migrated` (kept, never deleted) once every table is written. All credentials — including LLM API keys, previously plaintext on disk — are stored `enc:`-encrypted (ChaCha20-Poly1305, same `secrets.key` boundary as 0.9.x). `PUT /api/v1/config` persists to the DB when attached. `REVIEW_DISABLE_DB=1` is the escape hatch restoring full 0.9 behaviour (in-memory + file); a `DATABASE_URL` pointing at an unreachable PostgreSQL is a hard startup error, never a silent SQLite fallback. (`src/server/api/config/persist.rs`, `src/store/`, `src/cli/app.rs`)
+- **MR discussion context**: GitLab Note webhook payloads are ingested into `mr_discussions` in real time — idempotent upsert on `(platform, project, mr_iid, note_id)`, with a self-echo guard so our own published review reports are never re-ingested — and the pre-review flow additionally pulls discussions via the API and injects them into expert prompts, so follow-up reviews see prior human comments and review conclusions. (`src/server/gitlab/hooks.rs`, `src/server/api/review/discussion.rs`, `src/prompt/engine.rs`, `src/store/sqlx.rs`)
+- **Storage backend visibility**: `GET /api/v1/system/health` gains `storage_backend` (`postgresql` / `sqlite` / `disabled`); the Configuration page Advanced card shows it as a permanently-disabled read-only row (hidden fail-silent on health-check failure or an older server). (`src/server/api/system.rs`, `frontend/src/views/Configuration.vue`, `frontend/src/services/health.ts`, `frontend/src/i18n/locales/*` ×6)
+- **Review detail「完整评论」fallback**: when the aggregator produced no output (team reviews with `aggregated=None`), the full-comment tab falls back to the Lead consolidation TL;DR instead of rendering empty. (`src/server/api/review/task.rs`)
+
+### Fixed
+- **PostgreSQL placeholder rewriting (E2E-found release blocker)**: sqlx's `Any` driver passes SQL through verbatim — it does NOT translate `?` placeholders to `$n`, so every bound-parameter statement failed on PG with `42601 syntax error`. The store layer now rewrites top-level `?` to `$1..$n` (correctly skipping `?` inside string literals, quoted identifiers, and comments) at a single choke point (`SqlxStore::sql` / `adapt_sql`); SQLite passes through unchanged. (`src/store/placeholders.rs`, `src/store/sqlx.rs`)
+- **`llm_providers.temperature` declared `DOUBLE PRECISION`**: PG parses `REAL` as float4 while the store binds/decodes f64, so config read-back after restart failed with `mismatched types: f64 is not compatible with SQL type REAL` — silently leaving `GET /config` empty while `/health` stayed green (SQLite's 8-byte REAL is why tests never saw it). The column is now float8 on PG, same 8 bytes on SQLite. (`migrations/0001_init.sql`, `src/store/sqlx.rs`)
+- **`durationMs` wrap-around guard**: inverted timestamps (completed before started) now clamp to 0 instead of wrapping the u64 duration. (`src/server/task_queue.rs`)
+- **History list projection self-consistency**: rows whose materialized `project`/`repository` columns drifted from `source_meta` (failed back-fill, hand-seeded or legacy rows) are re-filled from the columns at read time, so a row matched by a `?project=X` filter no longer displays `project: null`. (`src/store/rows.rs`)
+- **SPA history-mode deep links no longer 404; static-dir miss is now diagnosable**: directly opening or refreshing a client-side route (`/history`, `/config`, `/reviews/42`) returned ServeDir's bare 404 — a `ServeDir::fallback` handler now serves `index.html` for extension-less non-`/api/` GET paths (200 + `no-cache, must-revalidate`, re-read from disk per request so an in-place upgrade never serves a stale copy), while unmatched `/api/` routes and file-like requests (extension in the last segment) keep their explicit 404 — serving HTML for a missing hashed asset would mask deploy breakage. Separately, `static_dir()` resolving `./frontend/dist` against the CWD silently degraded to the "Dashboard coming soon" placeholder when started outside the repository root; it now logs a WARN with the CWD and both checked paths. (`src/server/router.rs`, `tests/server/frontend.rs`)
+- **Project context for webhook-triggered first reviews (RENG-25)**: the lead-overview context gather treated `MRInfo.project_path` as a local filesystem path, but for webhook/API-triggered reviews it is the provider slug (`group/project`) and the server never clones the repository, so every first review of a repo with no local cache logged `failed to gather project context: Repository path does not exist` and degraded to an empty `ProjectContext`. The gatherer now only invokes the git-backed path when the path is an existing directory; otherwise (and on gather failure) it falls back to a partial context built from the reviewed diff's file list — first-time reviews get a real file tree without new network I/O. (`src/team/orchestrator/pipeline.rs`, `src/context/gather.rs`)
+- **Tolerant `GlobalReviewContext` YAML parsing (RENG-26)**: the lead-overview response was parsed with strict `serde_yaml_ng` on the raw LLM output — a ```` ```yaml ```` fence (backtick is a reserved YAML indicator) or tab indentation aborted the scanner with `found character that cannot start any token`, silently dropping the global context for the whole expert pass. Parsing is now a layered fallback: strict parse → parse after stripping code fences and normalizing tab indentation → parse of the first fenced YAML block only (reusing the shared output-parser helpers `clean_yaml` / `extract_first_fenced_yaml`). Total parse failure still degrades to no global context, unchanged. (`src/team/orchestrator/pipeline.rs`)
+- **History author column shows the commit author (RENG-27)**: the author column always showed the MR creator (e.g. the GitLab root account `Administrator`) instead of the person who wrote the commits. Author resolution now prefers the head commit's author and falls back to the MR creator: webhook parse lets `object_attributes.last_commit.author.name` win over `object_attributes.author.name`; GitLab `fetch_mr_info` does a best-effort `GET /repository/commits/<head_sha>` into the new `MRInfo.commit_author` (any failure degrades to `None`, never fails the review; GitHub path unchanged); `source_meta_from_mr_info` uses `commit_author` falling back to `pr_author`, blank treated as absent. No schema change — `author_name` lives in the `reviews.source_meta` JSON column, so existing history rows are untouched. (`src/git_provider/gitlab/client.rs`, `src/git_provider/github/client.rs`, `src/models/mod.rs`, `src/server/gitlab/hooks.rs`, `src/server/task_queue.rs`)
+- **Adjudication pass skips loudly when no local checkout exists (RENG-25)**: the adjudication pass assumed `MRInfo.project_path` is a local filesystem path, but for webhook/API-triggered reviews it is the provider slug and the server never clones the repository — every file load failed with `not readable from the local checkout` (INFO, one per file), no finding was actually adjudicated, yet the pipeline summary still logged `examined N findings`. Patch-only adjudication is unsafe (a unified diff carries only ±3 context lines, so the full-file ground-truth check is unsatisfiable and judging against it risks fail-closed drops), so the pass now skips explicitly: with no local checkout and candidates at or above the threshold it emits one WARN naming the reason and the number of findings passed through unadjudicated (kept unchanged, fail-open) and makes no LLM calls; the per-file skip inside a real checkout is elevated from INFO to WARN with the kept-finding count, and the pipeline summary says `candidates` instead of the misleading `examined`. CLI local reviews (real checkout) are unchanged. (`src/team/adjudicator.rs`, `src/team/orchestrator/pipeline.rs`)
+- **`/api/v1/reviews` pinned as the sole history list endpoint (RENG-29)**: `GET /api/v1/reviews/history` returned `400 Cannot parse task_id` because no such route exists — the request is captured by `/{task_id}` and fails UUID path-parameter validation. Routing is correct and every in-tree caller already uses `GET /reviews`, so the contract is pinned instead of expanding the API surface: `docs/rest-api.md` now documents `GET /reviews` as the only history list endpoint and `GET /reviews/:task_id` documents the 400 for non-UUID `task_id` alongside the existing 404; the regression test `reviews_history_subpath_is_not_a_route` locks the semantics end-to-end (list 200 envelope; `/reviews/history` 400 naming `task_id`). (`docs/rest-api.md`, `tests/server/reviews.rs`)
+- **Dashboard recent-reviews status stays on one line (RENG-30)**: the status column was 100px wide, but cell padding stacks (16px from `cellStyle` on the td plus Element Plus' default 12px on `.cell`), leaving ~44px of content width, and the default `.cell` `word-break: break-all` split `已完成` into `已完/成`. The column is widened to 108px to match the history table, the badge + label are wrapped in a flex cell, and the same truncation recipe is applied so over-long labels (e.g. ja `キャンセル済み`) ellipsize instead of wrapping. (`frontend/src/views/Dashboard.vue`)
+
+- **Store timestamp round-trip test is precision-aware and deterministic**: `review_row_codec_round_trip` previously asserted exact `DateTime` equality, which flaked on backends whose TEXT timestamp storage truncates sub-second precision. The test now compares with a precision-aware tolerance and fixed inputs, keeping the round-trip guarantee stable across SQLite/PostgreSQL. (`src/store/sqlx.rs`)
+- **Test assert messages no longer interpolate `api_key` (CodeQL)**: assertion failure messages in `persist.rs` and `sqlx.rs` tests embedded the API key value, tripping CodeQL's clear-text-logging rule; the messages now refer to the key without printing it. Behaviour of the assertions is unchanged. (`src/server/api/config/persist.rs`, `src/store/sqlx.rs`)
+- **`cargo audit` ignores RUSTSEC-2023-0071 with justification**: the rsa 0.9 Marvin-attack advisory fires on `sqlx-mysql`'s optional `rsa` dependency, which is pinned in Cargo.lock but never compiled (the workspace enables sqlx without `mysql`; verified via `cargo tree -i rsa`) and no patched rsa release exists. The ignore is scoped in `.cargo/audit.toml` with the full reasoning and a revisit condition. (`.cargo/audit.toml`)
+
+### Known issues
+- GitLab 19.x system hooks do not deliver MR note events (even with `note_events=true`): real-time comment ingestion requires a project-level webhook; under system-hook-only deployments the pre-review API pull covers the gap.
+- Deferred to 0.10.x: config-directory isolation is incomplete, the legacy `webhookSecret` masking policy needs alignment, and `/system/health` has no deep DB probe yet.
+
 ## [0.9.50] - 2026-09-02
 
 ### Added