feat: implement issue #846 — [Phase 2] Cache the LSP index and enforce the 30s cold-start SLA with auto-skip

.github/dependabot.yml

@@ -1,4 +1,3 @@
-version: 2
 updates:
   # GitHub Actions — version updates (keep actions current)
   - package-ecosystem: "github-actions"

.github/workflows/pr-review.yml

@@ -367,12 +367,38 @@ jobs:
       # threads the deep/audit-only MCP knobs into $GITHUB_ENV. A missing tool
       # degrades gracefully (::warning:: + skip) — it never fails the workflow.
       # Pins/candidate are overridable via repo variables; see docs/lsp-pilot.md.
+      #
+      # ── LSP index cache (epic #839, story #846, AC #1) ───────────────────
+      # Restore/save the LSP toolchain + index across ephemeral runs so a warm
+      # run skips re-indexing and stays inside the 30s P95 cold-start SLA. Keyed
+      # on runner OS + the pinned candidate/server versions + a hash of the
+      # indexed shell sources (the "relevant lockfile" for a Shell-only index):
+      # a source or version bump misses the exact key and rebuilds, while the
+      # restore-keys fallback still restores the binaries on a source-only
+      # change. Mirrors the Claude CLI binary cache pattern above and is
+      # SHA-pinned per the org standard (same actions/cache SHA as that step).
+      # Opt-in only (LSP_PILOT_ENABLED) — no cache work for off-pilot runs. The
+      # cache-hit outcome feeds setup-lsp-pilot.sh's cold-start instrumentation.
+      - name: Cache LSP index
+        id: lsp-index-cache
+        if: ${{ vars.LSP_PILOT_ENABLED == 'true' }}
+        uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
+        with:
+          path: |
+            ~/.local
+            ~/.cache/agent-lsp
+          key: lsp-index-${{ runner.os }}-${{ vars.LSP_CANDIDATE || 'agent-lsp' }}-${{ vars.AGENT_LSP_VERSION || 'v0.15.0' }}-bls${{ vars.BASH_LANGUAGE_SERVER_VERSION || '5.6.0' }}-${{ hashFiles('**/package-lock.json', 'scripts/**/*.sh') }}
+
       - name: Set up LSP pilot servers (opt-in)
         if: ${{ vars.LSP_PILOT_ENABLED == 'true' }}
         env:
           LSP_CANDIDATE: ${{ vars.LSP_CANDIDATE || 'agent-lsp' }}
           AGENT_LSP_VERSION: ${{ vars.AGENT_LSP_VERSION || 'v0.15.0' }}
           BASH_LANGUAGE_SERVER_VERSION: ${{ vars.BASH_LANGUAGE_SERVER_VERSION || '5.6.0' }}
+          # 'true' on an exact index-cache hit, 'false' otherwise (a miss or a
+          # restore-keys partial). setup-lsp-pilot.sh's cold-start record turns
+          # this into hit/miss so story #5 can report the cache hit-rate.
+          LSP_INDEX_CACHE_HIT: ${{ steps.lsp-index-cache.outputs.cache-hit == 'true' && 'true' || 'false' }}
         run: bash scripts/setup-lsp-pilot.sh
 
       - name: Enumerate candidate PRs

docs/lsp-pilot.md

@@ -43,18 +43,19 @@ see §2).
 
 ## 2. In-scope vs out-of-scope agent actions
 
-**In scope — finding-verification in the deep/audit tiers.** LSP is used as a
-*grounding/verification* step: before the reviewer posts a cross-file claim, it confirms the
-claim against real semantic context via:
+**In scope — LSP cold-start instrumentation and auto-skip in the deep/audit tiers.** This
+pilot phases in LSP by first establishing the cold-start infrastructure (timing, caching,
+and graceful degradation) so the agent can measure feasibility:
 
-- **find-references** (`textDocument/references`) — confirm "this breaks N callers" /
-  "this symbol is unused" against the actual reference set, not a textual `grep` match.
-- **diagnostics** (`textDocument/publishDiagnostics`) — confirm "this is undefined / this is
-  a type or syntax error" against the language server's own diagnostics.
+- **Cold-start SLA** — baseline cold-start time for the server's bring-up (target ≤30s P95).
+- **Index caching** — cache the language index across runs so only the first CI run (or after
+  source changes) incurs cold-start.
+- **Auto-skip on SLA miss** — if cold-start exceeds the SLA, gracefully skip LSP (warn, never
+  fail) and proceed with the review on base capabilities.
 
-This directly attacks the false-positive problem (textual `grep` matches a name in a string
-or comment; LSP resolves the *semantic* reference), which is what erodes trust in an
-auto-approving reviewer.
+Finding-verification (references, diagnostics) is considered as a *future* enhancement
+(epic Phase 2, [#843](https://github.com/petry-projects/.github-private/issues/843))
+once cold-start is established as viable.
 
 **Explicitly OUT of scope for this pilot:**
 
@@ -215,7 +216,8 @@ rebuild the gating, the graceful-degradation path, or the Token Cost Observatory
   `--mcp-config <file> --strict-mcp-config` and the merged `--allowed-tools` into the
   agentic tiers (`run_agentic` — deep/audit) and the rubber-duck tier (`run_duck`) via
   `REVIEW_MCP_CONFIG` / `REVIEW_MCP_ALLOWED_TOOLS`; the fast triage tier is intentionally
-  excluded. This pilot scopes LSP *use* to the deep/audit finding-verification surface (§2).
+  excluded. This pilot scopes LSP *use* to cold-start instrumentation in the deep/audit
+  tiers (§2); finding-verification is deferred to Phase 2 (epic #843).
   The LSP server is added as another entry in the same MCP config and its tools as another
   `REVIEW_MCP_ALLOWED_TOOLS` glob. See
   [`docs/initiatives/mcp-powered-review.md`](./initiatives/mcp-powered-review.md) §3 for the
@@ -226,13 +228,8 @@ rebuild the gating, the graceful-degradation path, or the Token Cost Observatory
   into a single `::warning::[mcp]` annotation while the review completes on base
   capabilities. The §3 SLA auto-skip is layered on top of this, not a parallel mechanism.
   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 —
-  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).
+- **What is genuinely new in this initiative:** the cold-start SLA + index-caching +
+  auto-skip on the LSP server's launch budget (#846).
 
 ---
 

prompts/deep-review.md

@@ -66,26 +66,6 @@ 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
 
@@ -133,14 +113,10 @@ Write a JSON object to `$OUTPUT_FILE`:
       "category": "...",
       "message": "...",
       "file": "path or null",
-      "line": "number or null",
-      "lsp_verification": "verified|unverifiable (OMIT unless step 8 applied)"
+      "line": "number or null"
     }
   ]
 }
 ```
 
-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,21 +34,6 @@ 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
 
@@ -86,16 +71,12 @@ Write a JSON object to `$OUTPUT_FILE`:
       "category": "...",
       "message": "...",
       "file": "path or null",
-      "line": "number or null",
-      "lsp_verification": "verified|unverifiable (OMIT unless step 6 applied)"
+      "line": "number or null"
     }
   ],
   "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`.