Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions .github/workflows/pr-review-trigger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,6 @@ on:
required: false
default: "false"
type: string
lsp_pilot_variant:
# LSP pilot A/B leg (epic #839, story #844, issue #1031). Drives the
# off-vs-on comparison on this repo's own PRs: dispatch the same PR with
# 'off' then 'on' to drop two joinable lsp_pilot_run records.
# on β†’ wire the LSP MCP + capture (lsp-on, B leg)
# off β†’ capture only, no LSP wiring (lsp-off, A control leg)
# none β†’ today's behaviour (defers to vars.LSP_PILOT_ENABLED)
description: "LSP pilot A/B leg: on | off | none"
required: false
default: "none"
type: choice
options:
- none
- "off"
- "on"
repository_dispatch:
# Triggered by the petry-projects/.github mention-listener (via
# pr-review-mention.yml). Payload: { pr_url, force_review }.
Expand All @@ -83,7 +68,4 @@ jobs:
pr_url: ${{ inputs.pr_url || '' }}
dry_run: ${{ inputs.dry_run || '' }}
force_review: ${{ inputs.force_review || '' }}
# Forward the LSP pilot A/B leg (empty for event triggers). 'none'/'' both
# mean today's behaviour on the reusable side.
lsp_pilot_variant: ${{ inputs.lsp_pilot_variant || '' }}
secrets: inherit
39 changes: 29 additions & 10 deletions tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,22 @@
LSP MCP (lsp-on/B leg), `off` captures without wiring (lsp-off/A control leg),
and the two LSP-wiring steps are gated so `off` skips them.

* AC #5 β€” the ring-0 trigger stub pr-review-trigger.yml declares and forwards
the new dispatch input so the A/B can be driven on this repo's own PRs.
* AC #5 (CORRECTED after incident 2026-07-03 / PR #1048) β€” the ring-0 trigger
stub pr-review-trigger.yml pins the reusable at `@pr-review/next`. Forwarding
`lsp_pilot_variant` to that channel BEFORE it declares the input makes every
review `startup_fail` (an undeclared `with:` key is an invalid workflow). So
the stub must NOT declare/forward the input until `pr-review/next` is advanced
(via cut-release.sh) to a commit that declares it. This guard now enforces that
channel-safety invariant (stub must not forward ahead of the pinned channel);
the original "must forward" form (#1034) is what took production down.

Off-pilot (neither var nor input set) must stay byte-for-byte unchanged (AC #4):
the exported env resolves to empty and the wiring steps stay skipped.

Re-enable procedure for the stub-driven A/B: (1) advance `pr-review/next` to a
commit that declares the reusable `lsp_pilot_variant` input, (2) re-add the stub
dispatch input + forwarding, and (3) flip check_trigger below back to requiring
them (see git history at #1034 for the requiring form).
"""
from __future__ import annotations

Expand Down Expand Up @@ -125,24 +136,32 @@ def check_reusable(fails: list[str]) -> None:


def check_trigger(fails: list[str]) -> None:
# CHANNEL-SKEW SAFETY (incident 2026-07-03, PR #1048). The ring-0 stub pins the
# reusable at `@pr-review/next`. A `with:` key the pinned reusable doesn't
# declare is an invalid workflow β†’ every review `startup_fail`s. The stub must
# therefore NOT declare or forward `lsp_pilot_variant` until that channel is
# advanced to a commit that declares it (then re-add both and flip this guard
# back β€” see the module docstring's re-enable procedure).
doc = _load(TRIGGER)
if not doc:
fails.append(f"{TRIGGER}: failed to parse YAML (document is None/empty)")
return
on = _on(doc)

wd_inputs = ((on.get("workflow_dispatch") or {}).get("inputs")) or {}
if INPUT not in wd_inputs:
fails.append(f"{TRIGGER}: workflow_dispatch.inputs.{INPUT} is not declared (AC #5)")
if INPUT in wd_inputs:
fails.append(
f"{TRIGGER}: workflow_dispatch.inputs.{INPUT} must NOT be declared while the "
f"stub pins a channel that lacks it (channel-skew guard β€” see #1048)"
)

job = _review_job(doc)
with_block = job.get("with") or {}
forwarded = str(with_block.get(INPUT, ""))
if not forwarded:
fails.append(f"{TRIGGER}: review job does not forward {INPUT} to the reusable (AC #5)")
elif f"inputs.{INPUT}" not in forwarded:
if INPUT in with_block:
fails.append(
f"{TRIGGER}: forwarded {INPUT} does not reference inputs.{INPUT} (got: {forwarded!r})"
f"{TRIGGER}: review job must NOT forward {INPUT} to the pinned reusable β€” "
f"forwarding an input @pr-review/next does not declare startup_fails every "
f"review (channel-skew guard β€” see #1048)"
)


Expand All @@ -165,7 +184,7 @@ def main() -> int:
return 1

print("PASS: pr-review.yml exports the capture gate + variant and gates LSP wiring; "
"pr-review-trigger.yml declares and forwards lsp_pilot_variant")
"pr-review-trigger.yml does not forward lsp_pilot_variant ahead of the pinned channel")
return 0


Expand Down
Loading