fix(#844): plumb LSP_PILOT_ENABLED env + decouple capture from LSP wiring so the pilot A/B runs in CI

[don-petry]

Epic: #839 · Story: #844 · Builds on: #961 (capture/emit) + #952 (extractor/harness)

Context

PR #961 added the pilot capture/emit logic to scripts/engine.sh + scripts/review-one-pr.sh (gated on the env LSP_PILOT_ENABLED), but only touched the scripts — not .github/workflows/pr-review.yml. Two gaps leave the feature inert / incomplete in real CI. This issue closes both so a production pr-review can emit a comparable lsp-off (A) and lsp-on (B) lsp_pilot_run record pair.

Gap 1 — the capture env var is never plumbed

_lsp_pilot_active() (engine.sh) and the trap in review-one-pr.sh gate on the env LSP_PILOT_ENABLED=true, but pr-review.yml only references the repo variable vars.LSP_PILOT_ENABLED (to gate the setup-lsp-pilot step, lines 378/387). vars.* are not automatically env vars, and neither the job env: block nor setup-lsp-pilot.sh exports LSP_PILOT_ENABLED to GITHUB_ENV. Result: with the repo var on, the LSP gets wired but no transcript is captured and no record is emitted (the unit tests pass only because they set the env directly).

Gap 2 — capture and LSP wiring can't be decoupled

Both the capture and the setup-lsp-pilot step key off the same vars.LSP_PILOT_ENABLED, and lpe_variant() returns lsp-on whenever REVIEW_MCP_CONFIG is wired. So a production run can only ever produce an lsp-on (B) record — there is no way to produce the lsp-off (A) control leg (capture on, LSP wiring off). The comparison needs both legs from the same pipeline.

Acceptance criteria

1. Plumb the capture gate. The review job in pr-review.yml must export LSP_PILOT_ENABLED as an env var to the step(s) that run the engine / review-one-pr.sh, so _lsp_pilot_active() actually sees it. When capture is enabled and a review runs, a kind:"lsp_pilot_run" record is emitted to TOKEN_LOG_FILE (already uploaded as the token-usage-<run_id> artifact).
2. Decouple capture from LSP wiring. Add an independent control so a run can enable capture/emit WITHOUT wiring the LSP MCP (→ lsp-off A leg) and another that enables capture WITH LSP wiring (→ lsp-on B leg). Suggested shape: a workflow_dispatch input (with a matching repo-var default), e.g. lsp_pilot_variant: off | on | none — on runs setup-lsp-pilot (wires REVIEW_MCP_CONFIG) + capture; off runs capture only (skips setup-lsp-pilot, no REVIEW_MCP_CONFIG); none/unset = today's behaviour. Preserve the existing vars.LSP_PILOT_ENABLED meaning for anyone already using it.
3. Both legs join. Dispatching the same PR twice (variant off, then on) must yield two lsp_pilot_run records with the same pr key and variants lsp-off / lsp-on, that scripts/lsp_pilot_compare.sh renders as an off-vs-on table.
4. Off-pilot unchanged. With neither the variable nor the input set, pr-review.yml and the engine behave byte-for-byte as before — no stream capture, no records, no extra flags. The 5 consumer repos must be unaffected.
5. Thin-caller forwarding. If a new dispatch input is added, forward it through pr-review-trigger.yml (the ring-0 trigger stub) within its allowed-input contract so the A/B can be driven on this repo's own PRs. Respect the thin-caller rules in AGENTS.md.
6. Tests + hygiene. Coverage for the new plumbing/decoupling; shellcheck --severity=warning -x scripts/*.sh clean; pr-review.yml validates as YAML; existing test_engine_lsp_pilot / test_lsp_pilot_emit / test_lsp_pilot_measure suites stay green. Preserve all action/SHA pinning.

Out of scope

• Enabling the pilot in production / setting repo variables — owner-gated (that's the run, not this change).
• Any change to the lsp_pilot_measure.sh / lsp_pilot_compare.sh record schema.
• The go/no-go decision ([Phase 3] Record the human go/no-go decision and fleet-wide recommendation #845).

Done when

Dispatching the ring-0 pr-review on one PR with the pilot variant off then on drops two joinable lsp_pilot_run records (lsp-off + lsp-on) into their token-usage artifacts, and scripts/lsp_pilot_compare.sh renders a real off-vs-on table — no synthetic data and no manual env hacks.

[github-actions]

Thanks for the thorough write-up and clear acceptance criteria — this is a well-scoped issue! Before we dig in: for Gap 2's proposed lsp_pilot_variant dispatch input, do you have a preference on whether the off leg should still respect vars.LSP_PILOT_ENABLED as a fallback default, or should the repo var exclusively gate the existing behaviour so the new input is fully independent?

[don-petry]

Dev-Lead Implementation Plan

Issue: #1031 — fix(#844): plumb LSP_PILOT_ENABLED env + decouple capture from LSP wiring so the pilot A/B runs in CI

Scope

Export the LSP_PILOT_ENABLED capture gate as a real env var in pr-review.yml, and add an lsp_pilot_variant (on|off|none) control that decouples capture from LSP-MCP wiring so a production review can emit both an lsp-off (A) and lsp-on (B) lsp_pilot_run record from the same pipeline — with off-pilot behaviour byte-for-byte unchanged.

Root cause notes

• Gap 1: the review step gates capture on the env LSP_PILOT_ENABLED, but the workflow only referenced the repo var vars.LSP_PILOT_ENABLED (never exported to the job env) → capture never ran in CI.
• Gap 2: lpe_variant() returns lsp-on whenever REVIEW_MCP_CONFIG is a readable file. Because engine.sh auto-defaults REVIEW_MCP_CONFIG to the committed .github/review-mcp.json (present in this repo) and review-one-pr.sh sources engine.sh, the off (A) leg would be mislabelled lsp-on. Fix: an explicit LSP_PILOT_VARIANT (on|off) that is authoritative for the leg label, falling back to REVIEW_MCP_CONFIG detection when unset (preserves legacy vars.LSP_PILOT_ENABLED users).

Implementation Checklist

• scripts/lsp_pilot_emit.sh — lpe_variant() honours explicit LSP_PILOT_VARIANT=on|off (authoritative), else falls back to REVIEW_MCP_CONFIG detection; lpe_candidate() follows.
• .github/workflows/pr-review.yml — export job-env LSP_PILOT_ENABLED (on when vars.LSP_PILOT_ENABLED == 'true' OR variant is on/off) and LSP_PILOT_VARIANT; gate the two LSP-wiring steps (Cache LSP index, Set up LSP pilot servers) on vars.LSP_PILOT_ENABLED == 'true' || variant == 'on'; add lsp_pilot_variant workflow_call input.
• .github/workflows/pr-review-trigger.yml — add lsp_pilot_variant workflow_dispatch input and forward it (thin-caller contract).
• .github/workflows/test-dev-lead.yml — run the new plumbing test + trigger on pr-review*.yml path changes.

Tests to Write

• tests/dev-lead/unit/test_lsp_pilot_emit.bats — LSP_PILOT_VARIANT=off forces lsp-off even with REVIEW_MCP_CONFIG set; on forces lsp-on with it unset; none/unset falls back; candidate + emit-record A-leg decoupling.
• tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py — pr-review.yml exports the capture gate + variant env and gates wiring on on; pr-review-trigger.yml declares and forwards lsp_pilot_variant.

Test Command

bats tests/dev-lead/unit/ && python3 tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py && bash .dev-lead/scripts/dev-lead-lint.sh

[don-petry]

Dev-Lead: Implementation Complete

Plan Execution

• Gap 1 — plumbed the capture gate: pr-review.yml now exports the ENV LSP_PILOT_ENABLED to the review job (the engine + emit trap read the env, not the repo var), so a production review captures the stream-json transcript and appends a kind:"lsp_pilot_run" record to TOKEN_LOG_FILE (already uploaded as token-usage-<run_id>).
• Gap 2 — decoupled capture from LSP wiring via a new lsp_pilot_variant: on|off|none control (with a vars.LSP_PILOT_VARIANT repo-var default). on wires the LSP MCP + captures (lsp-on/B); off captures only, skips wiring (lsp-off/A); none/unset = today's behaviour.
• Made lpe_variant() honour explicit LSP_PILOT_VARIANT as authoritative — this is required because engine.sh auto-defaults REVIEW_MCP_CONFIG to the committed .github/review-mcp.json (present here) and review-one-pr.sh sources engine.sh, which would otherwise mislabel the A leg as lsp-on. Falls back to REVIEW_MCP_CONFIG detection when unset (preserves the legacy vars.LSP_PILOT_ENABLED path).
• AC5 — forwarded lsp_pilot_variant through the ring-0 trigger stub pr-review-trigger.yml.
• AC4 — verified byte-for-byte off-pilot: with neither the var nor the input set, the exported env resolves to empty and both LSP-wiring steps stay skipped (truth-table verified).

Resolved leg truth table (verified)

case	LSP_PILOT_ENABLED	LSP wiring	LSP_PILOT_VARIANT	→ leg
off-pilot (nothing)	''	skip	''	unchanged
legacy vars.LSP_PILOT_ENABLED=true	true	run	''	lsp-on (fallback)
dispatch off	true	skip	off	lsp-off (A)
dispatch on	true	run	on	lsp-on (B)
dispatch none	''	skip	none	unchanged

Test Results

# dev-lead unit (bats) — all pass
tests/dev-lead/unit/  → 678 ok, 0 failures
  incl. test_lsp_pilot_emit.bats: 22 ok (7 new: LSP_PILOT_VARIANT authority + A/B emit decoupling)
       test_engine_lsp_pilot.bats / test_lsp_pilot_measure.bats: green

# lsp pilot report/compare (bats) — green
tests/lsp_pilot_compare.bats / tests/lsp_pilot_report.bats

# integration (python) — all pass
test_pr_review_lsp_pilot_plumbing.py  PASS  (new)
test_caller_permissions.py / test_auto_rebase_stub.py / test_dependency_audit_stub.py
test_workflow_toplevel_permissions.py / test_dev_lead_stub.sh  PASS

# required lint gate
bash .dev-lead/scripts/dev-lead-lint.sh  → all checks passed
  shellcheck --severity=warning -x on 85 scripts (incl. lsp_pilot_emit.sh): clean
  agent profiles + CODEOWNERS: OK

# YAML validity
pr-review.yml / pr-review-trigger.yml / test-dev-lead.yml  → parse OK

Files Changed

• .github/workflows/pr-review.yml: export LSP_PILOT_ENABLED (capture gate) + LSP_PILOT_VARIANT job env; add lsp_pilot_variant workflow_call input; gate the two LSP-wiring steps (Cache LSP index, Set up LSP pilot servers) on vars.LSP_PILOT_ENABLED == 'true' || variant == 'on'.
• .github/workflows/pr-review-trigger.yml: add lsp_pilot_variant workflow_dispatch input (choice: none/off/on) and forward it to the reusable.
• scripts/lsp_pilot_emit.sh: lpe_variant() treats explicit LSP_PILOT_VARIANT=on|off as authoritative, else falls back to REVIEW_MCP_CONFIG detection.
• tests/dev-lead/unit/test_lsp_pilot_emit.bats: 7 new tests for the variant authority + A/B emit decoupling; hermetic-unset of LSP_PILOT_VARIANT.
• tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py: new guard for the capture-gate export, variant decoupling, wiring-step gates, and trigger-stub forwarding.
• .github/workflows/test-dev-lead.yml: run the new plumbing test as a job; add pr-review*.yml to the trigger paths.

Notes

• Record schema (lsp_pilot_measure.sh / lsp_pilot_compare.sh) is untouched — out of scope. The A/B pair still joins on the same pr key with variants lsp-off/lsp-on, which lsp_pilot_compare.sh renders unchanged.
• Enabling the pilot in production (setting repo variables) is owner-gated and out of scope — this change only makes the A/B runnable on dispatch.
• Both .github/mcp/lsp.json and .github/review-mcp.json exist in this repo, so a real on run wires the LSP config (overriding the auto-default) and a real off run keeps the baseline yet is correctly labelled lsp-off.