feat: implement issue #843 — [Phase 2] Add the LSP finding-verification step to the deep/audit pr-review tiers

docs/lsp-pilot.md

@@ -228,7 +228,11 @@ rebuild the gating, the graceful-degradation path, or the Token Cost Observatory
   See [`docs/initiatives/mcp-powered-review.md`](./initiatives/mcp-powered-review.md) §4.
 - **What is genuinely new in this initiative:** (a) the cold-start SLA + index-caching +
   auto-skip on the LSP server's launch budget (#846), and (b) the finding-verification step
-  that calls find-references / diagnostics before posting a cross-file finding (#843).
+  that calls find-references / diagnostics before posting a cross-file finding (#843 —
+  shipped: the deep/audit prompts ask the model to annotate each grounded finding with
+  `lsp_verification`, and [`scripts/lib/lsp-verification.sh`](../scripts/lib/lsp-verification.sh)
+  enforces it — downgrading + annotating `unverifiable` findings and emitting each outcome to
+  the Token Cost Observatory JSONL. It is inert when LSP is unwired or degraded).
 
 ---
 

prompts/deep-review.md

@@ -66,6 +66,26 @@ enumeration. No actions on other PRs.
      review because MCP was unavailable, and never fabricate a scan result.
 6. Fetch linked issues if any.
 7. Check `statusCheckRollup` for CI status.
+8. **LSP finding-verification (only when the `mcp__lsp__*` navigation tools are
+   available).** These tools are exposed only when the LSP pilot is enabled; if
+   they are not present, skip this step entirely and review as usual. When they
+   are present, before you report any finding that makes a **cross-file or
+   semantic claim** — e.g. "X is undefined", "this breaks N callers", "this
+   symbol is unused", "this is a type/syntax error" — ground the claim against
+   real semantic context instead of a textual `grep` match:
+   - use `mcp__lsp__find_references` to confirm a "breaks N callers" / "unused
+     symbol" claim against the actual reference set;
+   - use `mcp__lsp__get_diagnostics` to confirm an "undefined" / "type or syntax
+     error" claim against the language server's own diagnostics.
+   Then annotate that finding with an `"lsp_verification"` field:
+   - `"verified"` — LSP confirmed the claim;
+   - `"unverifiable"` — LSP could not ground the claim. **Do not drop it and do
+     not post it as confident**: lower its `severity` one level; the
+     verification step also tags it `[lsp: unverifiable]` so the outcome is
+     auditable.
+   Findings that make no cross-file/semantic claim (style, docs, etc.) need no
+   `lsp_verification` field. Never fail or block a review because an LSP tool was
+   unavailable, and never fabricate a verification result.
 
 ## Risk classification
 
@@ -113,10 +133,14 @@ Write a JSON object to `$OUTPUT_FILE`:
       "category": "...",
       "message": "...",
       "file": "path or null",
-      "line": "number or null"
+      "line": "number or null",
+      "lsp_verification": "verified|unverifiable (OMIT unless step 8 applied)"
     }
   ]
 }
 ```
 
+Include `lsp_verification` **only** on a finding you grounded via the LSP tools
+in step 8 (`verified` or `unverifiable`); omit it otherwise.
+
 Write with `cat > "$OUTPUT_FILE" <<'JSON' ... JSON`. Ensure it parses with `jq`.

prompts/security-audit.md

@@ -34,6 +34,21 @@ reviewer, called only for PRs with real concerns.
 4. Fetch linked issues if any.
 5. Read any CONTRIBUTING.md, AGENTS.md, CODEOWNERS in the repo to check
    standards compliance (fetch via `gh api`).
+6. **LSP finding-verification (only when the `mcp__lsp__*` navigation tools are
+   available).** These tools are exposed only when the LSP pilot is enabled; if
+   they are not present, skip this step entirely and audit as usual. When they
+   are present, before you report any finding that makes a **cross-file or
+   semantic claim** — e.g. "X is undefined", "this breaks N callers", "this
+   symbol is unused", "this is a type/syntax error" — ground it against real
+   semantic context with `mcp__lsp__find_references` (reference set) or
+   `mcp__lsp__get_diagnostics` (language-server diagnostics) rather than a textual
+   `grep` match. Annotate that finding with an `"lsp_verification"` field:
+   `"verified"` if LSP confirmed it, or `"unverifiable"` if LSP could not ground
+   it — and for `unverifiable` lower its `severity` one level rather than dropping
+   it (the verification step also tags it `[lsp: unverifiable]` so the outcome is
+   auditable). Findings with no cross-file/semantic claim need no field. Never
+   fail the audit because an LSP tool was unavailable, and never fabricate a
+   verification result.
 
 ## Your focus
 
@@ -71,12 +86,16 @@ Write a JSON object to `$OUTPUT_FILE`:
       "category": "...",
       "message": "...",
       "file": "path or null",
-      "line": "number or null"
+      "line": "number or null",
+      "lsp_verification": "verified|unverifiable (OMIT unless step 6 applied)"
     }
   ],
   "sonnet_findings_confirmed": ["<indices of deep review findings you agree with>"],
   "sonnet_findings_dismissed": ["<indices you disagree with, with reason>"]
 }
 ```
 
+Include `lsp_verification` **only** on a finding you grounded via the LSP tools
+in step 6 (`verified` or `unverifiable`); omit it otherwise.
+
 Write with `cat > "$OUTPUT_FILE" <<'JSON' ... JSON`. Ensure it parses with `jq`.

scripts/lib/lsp-verification.sh

@@ -0,0 +1,141 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# LSP finding-verification step — epic #839 (LSP pilot), story #843.
+# Contract: docs/lsp-pilot.md §2 (find-references / diagnostics grounding).
+#
+# This is the one genuinely-new piece of the LSP pilot (docs/lsp-pilot.md §6):
+# before a deep/audit finding that asserts a cross-file/semantic claim (e.g.
+# "undefined", "breaks N callers", "unused symbol") is posted, the reviewer
+# grounds it against the LSP navigation tools (find_references / get_diagnostics).
+# Only the model can call the MCP `mcp__lsp__*` tools, so it annotates each such
+# finding in its JSON output with an `lsp_verification` field ("verified" |
+# "unverifiable"); this shell layer ENFORCES that annotation deterministically:
+#
+#   - A finding LSP could not ground ("unverifiable") is DOWNGRADED one severity
+#     level and ANNOTATED `[lsp: unverifiable]` — never silently dropped and
+#     never posted as a confident finding (AC #2).
+#   - Each verified/unverifiable outcome is emitted to the Token Cost Observatory
+#     JSONL so the comparison harness can compute the false-positive-rate delta
+#     (AC #4) — via emit_verification_record in token-metrics.sh.
+#   - The whole step is INERT (findings file byte-for-byte unchanged, nothing
+#     emitted) when the LSP server is absent or degraded, so the review proceeds
+#     exactly as today (AC #3). This reuses the same MCP connect/degradation
+#     signal `_emit_mcp_failure_warning` keys on (scripts/engine.sh).
+#
+# Sourced by scripts/review-one-pr.sh after engine.sh and token-metrics.sh.
+
+# _lsp_failure_pattern
+# The MCP connection/init-failure regex used to detect a degraded LSP server.
+# Prefer engine.sh's single source of truth (_mcp_failure_pattern) when this lib
+# is sourced alongside it; otherwise fall back to an equivalent local pattern so
+# the lib stays usable (and unit-testable) on its own.
+_lsp_failure_pattern() {
+  if declare -f _mcp_failure_pattern >/dev/null 2>&1; then
+    _mcp_failure_pattern
+    return
+  fi
+  local _pat
+  _pat='mcp server [^[:space:]]*[[:space:]]*("[^"]*"[[:space:]]*)?(failed|error)'
+  _pat="$_pat"'|failed to (connect|initialize|reconnect|start)[^.]*mcp'
+  _pat="$_pat"'|mcp[^.]*(connection|initializ)[^.]*(fail|error)'
+  _pat="$_pat"'|could not (connect to|start) mcp server'
+  printf '%s' "$_pat"
+}
+
+# _lsp_downgrade_severity <severity>
+# One step toward "info" on the info|minor|major|critical ladder. Bottoms out at
+# info; unknown values collapse to info (safe — never escalates).
+_lsp_downgrade_severity() {
+  case "$1" in
+    critical) printf 'major' ;;
+    major)    printf 'minor' ;;
+    *)        printf 'info'  ;;
+  esac
+}
+
+# lsp_verification_active [cli_output_file...]
+# Returns 0 (active) iff the LSP navigation tools are wired AND not degraded:
+#   - REVIEW_MCP_CONFIG points at a readable file,
+#   - REVIEW_MCP_ALLOWED_TOOLS permits at least one mcp__lsp__ tool (so this is
+#     the LSP pilot, not e.g. a Context7-only run),
+#   - none of the given CLI-output files show an MCP connect/init failure.
+# Returns 1 (inert) otherwise. The CLI-output scan is the degradation gate: a
+# server that failed to connect means "skip verification, review as today".
+lsp_verification_active() {
+  [ -n "${REVIEW_MCP_CONFIG:-}" ] || return 1
+  [ -f "${REVIEW_MCP_CONFIG}" ] && [ -r "${REVIEW_MCP_CONFIG}" ] || return 1
+  case "${REVIEW_MCP_ALLOWED_TOOLS:-}" in
+    *mcp__lsp__*) : ;;
+    *) return 1 ;;
+  esac
+
+  if [ "$#" -gt 0 ]; then
+    local f present=()
+    for f in "$@"; do [ -f "$f" ] && present+=("$f"); done
+    if [ "${#present[@]}" -gt 0 ] \
+       && grep -qiE "$(_lsp_failure_pattern)" "${present[@]}" 2>/dev/null; then
+      return 1
+    fi
+  fi
+  return 0
+}
+
+# apply_lsp_verification <findings_json> <tier> [cli_output_file...]
+# Enforces the LSP finding-verification contract on a review verdict file in
+# place. No-op (file untouched, nothing emitted) when verification is inert —
+# LSP unwired/degraded, jq missing, or the file is not a {findings:[...]} object.
+apply_lsp_verification() {
+  local file="${1:-}" tier="${2:-deep}"
+  if [ "$#" -ge 2 ]; then shift 2; else shift "$#"; fi   # remaining args: CLI output files
+  [ -f "$file" ] || return 0
+  command -v jq >/dev/null 2>&1 || return 0
+  jq -e 'type == "object" and (.findings | type == "array")' "$file" >/dev/null 2>&1 || return 0
+  lsp_verification_active "$@" || return 0
+
+  local workflow="${TOKEN_WORKFLOW:-unknown}" context="${PR_URL:-}"
+
+  # Emit one verification record per annotated finding (before rewriting, so the
+  # original severity is the recorded severity_before).
+  if declare -f emit_verification_record >/dev/null 2>&1; then
+    local idx outcome sev cat sev_after
+    while IFS=$'\t' read -r idx outcome sev cat; do
+      [ -n "$idx" ] || continue
+      if [ "$outcome" = "unverifiable" ]; then
+        sev_after="$(_lsp_downgrade_severity "$sev")"
+      else
+        sev_after="$sev"
+      fi
+      emit_verification_record "$workflow" "$tier" "$context" \
+        "$idx" "$cat" "$sev" "$sev_after" "$outcome" || true
+    done < <(jq -r '
+      .findings | to_entries[]
+      | (.value.lsp_verification // "") as $v
+      | select($v == "verified" or $v == "unverifiable")
+      | [ (.key | tostring), $v, (.value.severity // "info"), (.value.category // "") ]
+      | @tsv
+    ' "$file" 2>/dev/null)
+  fi
+
+  # Downgrade + annotate every unverifiable finding (atomic rewrite).
+  local tmp; tmp="$(mktemp "${file}.lspv.XXXXXX")" || return 0
+  if jq '
+      def downgrade:
+        if . == "critical" then "major"
+        elif . == "major" then "minor"
+        else "info" end;
+      .findings |= map(
+        if (.lsp_verification // "") == "unverifiable" then
+            .severity = ((.severity // "info") | downgrade)
+          | .message = ((.message // "")
+              + (if (.message // "") | test("\\[lsp: unverifiable\\]")
+                 then "" else " [lsp: unverifiable]" end))
+        else . end
+      )
+    ' "$file" > "$tmp" 2>/dev/null && [ -s "$tmp" ]; then
+    mv "$tmp" "$file"
+  else
+    rm -f "$tmp"
+  fi
+  return 0
+}

scripts/lib/token-metrics.sh

@@ -117,6 +117,52 @@ emit_token_record() {
   printf '%s\n' "$record" >> "$TOKEN_LOG_FILE" 2>/dev/null || true
 }
 
+# emit_verification_record <workflow> <tier> <context> <finding_index>
+#                          <category> <severity_before> <severity_after> <outcome>
+# Appends one finding-verification record to TOKEN_LOG_FILE on the SAME JSONL
+# channel as token records (story #843, epic #839). The record is discriminated
+# by kind:"finding_verification" so the comparison harness (story 2) can compute
+# the false-positive-rate delta and the cost report can skip it (it is not a
+# priced token-usage call — see scripts/token_report.sh `annotate_records`).
+# No-op when TOKEN_LOG_FILE is unset; swallows I/O errors so it never aborts a run.
+emit_verification_record() {
+  [ -n "${TOKEN_LOG_FILE:-}" ] || return 0
+
+  local workflow="$1" tier="$2" context="$3" finding_index="$4"
+  local category="$5" severity_before="$6" severity_after="$7" outcome="$8"
+
+  local ts run_id record
+  ts=$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo "")
+  run_id="${GITHUB_RUN_ID:-}"
+
+  record=$(jq -cn \
+    --arg ts "$ts" \
+    --arg workflow "$workflow" \
+    --arg tier "$tier" \
+    --arg context "$context" \
+    --arg finding_index "$finding_index" \
+    --arg category "$category" \
+    --arg severity_before "$severity_before" \
+    --arg severity_after "$severity_after" \
+    --arg outcome "$outcome" \
+    --arg run_id "$run_id" \
+    '{
+      kind: "finding_verification",
+      ts: $ts,
+      workflow: $workflow,
+      tier: $tier,
+      context: $context,
+      finding_index: ($finding_index | tonumber? // $finding_index),
+      category: $category,
+      severity_before: $severity_before,
+      severity_after: $severity_after,
+      outcome: $outcome,
+      run_id: $run_id
+    }' 2>/dev/null) || return 0
+
+  printf '%s\n' "$record" >> "$TOKEN_LOG_FILE" 2>/dev/null || true
+}
+
 # ─────────────────────────────────────────────────────────────────────────────
 # Real-usage capture (replaces the char/4 estimate when an engine reports usage)
 #

scripts/review-one-pr.sh

@@ -41,6 +41,12 @@ source "$SCRIPT_DIR/lib/review-registry.sh"
 # Gated default-off behind DOWNSTREAM_IMPACT_ENABLED (Story 5).
 # shellcheck source=lib/downstream-impact.sh
 source "$SCRIPT_DIR/lib/downstream-impact.sh"
+# LSP finding-verification (epic #839, story #843): apply_lsp_verification grounds
+# deep/audit cross-file findings against LSP nav tools, downgrading/annotating the
+# ones LSP cannot ground and emitting each outcome to the token JSONL. Inert when
+# the LSP MCP server is unwired or degraded (review unchanged).
+# shellcheck source=lib/lsp-verification.sh
+source "$SCRIPT_DIR/lib/lsp-verification.sh"
 
 PR_URL="${1:?usage: review-one-pr.sh <pr-url>}"
 export PR_URL
@@ -847,6 +853,13 @@ fi
 # Wait for duck to finish (deep succeeded)
 [ -n "$DUCK_PID" ] && wait $DUCK_PID || true
 
+# LSP finding-verification (story #843): ground the deep tier's cross-file
+# findings before they are synthesized/posted. Inert (no change) unless the LSP
+# MCP server is wired and connected; the deep CLI output is scanned so a degraded
+# server skips verification. Operates in place on deep.json.
+apply_lsp_verification "$OUTPUT_FILE" "deep" \
+  /tmp/cascade/deep-stdout.txt /tmp/cascade/deep.log || true
+
 DEEP_DECISION=$(jq -r '.decision' "$OUTPUT_FILE")
 DEEP_RISK=$(jq -r '.risk' "$OUTPUT_FILE")
 echo "    [tier2] deep: decision=$DEEP_DECISION risk=$DEEP_RISK"
@@ -976,6 +989,12 @@ if [ ! -s "$OUTPUT_FILE" ] || ! jq empty "$OUTPUT_FILE" 2>/dev/null; then
   exit 1
 fi
 
+# LSP finding-verification (story #843): ground the audit tier's cross-file
+# findings before the final verdict is posted. Inert unless the LSP MCP server is
+# wired and connected (audit CLI output scanned for a degraded server).
+apply_lsp_verification "$OUTPUT_FILE" "audit" \
+  /tmp/cascade/audit-stdout.txt /tmp/cascade/audit.log || true
+
 AUDIT_DECISION=$(jq -r '.decision' "$OUTPUT_FILE")
 AUDIT_RISK=$(jq -r '.risk' "$OUTPUT_FILE")
 echo "    [tier3] decision=$AUDIT_DECISION risk=$AUDIT_RISK"

scripts/token_report.sh

@@ -76,7 +76,9 @@ annotate_records() {
   local dir="$1"
   local files=("$dir"/*.jsonl)
   [ -e "${files[0]}" ] || return 0   # no JSONL files → no rows
-  jq -r 'select(type == "object") | [
+  jq -r 'select(type == "object")
+    | select((.kind // "token_usage") == "token_usage")
+    | [
       (.repo // "unknown"), (.workflow // "unknown"), (.tier // "-"), (.model // "-"),
       (.input_tokens // 0), (.cache_read_tokens // 0), (.output_tokens // 0),
       (.ts // "-"), (.context // ""), (.cache_creation_tokens // 0)