Skip to content

fix(source-control): bind review evidence on review plugin changes - #10027

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/lane-source-review-plugin
Oct 25, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/lane-source-review-plugin

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: owner decision Q-LSC in the poteto session, 2026-10-25 ("go w/ recommended"). It unsticks lane PRs #9890, #9831, #9821, #9806 and #9800.

Summary

pr-merge's review-evidence binder no longer unbinds both review lanes when a PR changes plugins/review/. The review plugin never comes from the PR's head: the reusable review workflow stages it read-only from the claude-code-plugins default branch's tip. So a PR's changes to the plugin cannot change the review it gets.

Fix

  • babysit_review_evidence.py: plugins/review/ leaves LANE_SOURCE_PREFIXES. Every other lane source stays, including .github/workflows/, the standards and criteria docs, .claude-lane/, .claude*, CLAUDE.md and .gitattributes. The comment says why the plugin is not listed and names the test that pins it.
  • New scripts/ci-review-plugin-source.test.sh: fails if either review caller (pr-review-hosted.yml, pr-review-security-hosted.yml) stops calling melodic-software/ci-workflows' pr-review-cli.yml at a 40-hex SHA, or passes any plugin input or plugin path from the checkout. A PR that repoints a caller still unbinds, because .github/workflows/ stays a lane source.

Verification

  • Evidence that the plugin comes from main: pr-review-cli.yml@4fbceb4b.
    • L76-78: "The review plugin comes from the claude-code-plugins default branch's tip, never the head".
    • L934-950: the staging step, which also fails a head that holds .lane.
    • plugin-dirs: .lane/plugins/review at L1178 and L2265.
  • python3 -m unittest discover -s tests -t . (babysit-prs scripts): 2897 OK.
    • New: a plugins/review change binds both lanes (5 paths, including the bare directory and .claude-plugin/plugin.json). It failed all 5 subtests on the old code.
    • New: a plugins/review change beside a lane source (a caller workflow, docs/standards, .claude-lane, plugins/review/CLAUDE.md, plugins/review/.claude/settings.json) still unbinds.
  • scripts/ci-review-plugin-source.test.sh: 10 passed. Seven bad-shape fixtures are rejected: tag ref, short SHA, another repository, plugin-dirs, a plugin path in another input, no call, and two calls.
  • scripts/check-affected.sh --tests: 14 passed.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01LY6PpXaikB8J2tRLeypMmn

kyle-sexton and others added 2 commits October 25, 2026 10:10
The reusable pr-review-cli.yml stages the review plugin from
claude-code-plugins' default-branch tip, never the head, so a pull
request that changes plugins/review/ cannot change its own review.
Drop plugins/review/ from LANE_SOURCE_PREFIXES and pin the reason with
scripts/ci-review-plugin-source.test.sh, which fails if either review
caller stops calling that reusable at a commit SHA or passes it a
plugin path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LY6PpXaikB8J2tRLeypMmn
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 25, 2026 14:18
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T14:20:50.655284Z 0f1e739 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 0f1e739b18e7ce8fce7c26dfa236aaffd8732a04: 0 finding(s), 0 as inline comments.

Security review of PR #10027 at 0f1e739: no security findings.

critical-findings: 0

What the change does. plugins/review/ is removed from LANE_SOURCE_PREFIXES in babysit_review_evidence.py. A PR that touches the review plugin therefore no longer unbinds both review lanes (lane-source-changed). The other lane-source paths stay: .github/workflows/, docs/standards/, the exception register, the untrusted-content contract, .claude-lane/, any .claude* path segment, CLAUDE.md, AGENTS.md, REVIEW.md, .mcp.json and .gitattributes. A new structure test, scripts/ci-review-plugin-source.test.sh, pins the assumption the change rests on.

Why this is the instruction-surface and trust-boundary question. The binder decides whether a review record proves the default-branch lane reviewed the head. Narrowing the list is safe only if a PR's copy of plugins/review/ can never feed its own review. I checked that against the checkout:

  • Both callers, .github/workflows/pr-review-hosted.yml:172 and .github/workflows/pr-review-security-hosted.yml:177, call melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml pinned to the 40-hex SHA 4fbceb4b…. A search of both files for plugin found no plugin-path input.
  • The lane skill itself loads from .lane/plugins/review, which matches the claim that the plugin is staged outside the PR head.
  • I could not read the reusable at the pinned SHA, so the claim that it stages the plugin from the default branch's tip rests on the PR description's citation, not on my own check. The pin plus the new test keep a later change from silently breaking the claim.

Bypass paths checked.

  • A PR that repoints a caller, drops the SHA pin, or adds a plugin input must edit .github/workflows/, which still unbinds. The new test also fails in that case.
  • A PR that changes the test script cannot widen what the binder trusts, because the binder reads only LANE_SOURCE_PREFIXES. A PR that edits that file is itself visible as a change to the lane's own source, and the default-branch lane reviews it.
  • plugins/review/CLAUDE.md and plugins/review/.claude/** still unbind through the segment and name rules. The new tests cover both.
  • The test script checks plugin-like keys and values only under with:. It does not cover env: or secrets: passthrough. No exploit path exists in this diff, and it is a coverage limit, not a finding.

Manifests. The diff changes no plugin.json or marketplace.json, so there is no manifest <field> line. The .changes/ file is a changelog entry only.

Spend. The change adds a test script and does not affect turn limits, review frequency or billing, so there is no cost hold.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 0f1e739b18e7ce8fce7c26dfa236aaffd8732a04: 0 finding(s), 0 as inline comments.

No findings. I read the diff, the PR context and both caller workflows. I did not run any tests.

  • Binder change: plugins/review/ is removed from LANE_SOURCE_PREFIXES. The other lane sources still unbind: .github/workflows/, docs/standards/, .claude-lane/, any .claude path segment, and CLAUDE.md, REVIEW.md and .gitattributes. plugins/review/.claude-plugin/plugin.json is not caught by the .claude segment check, which matches whole segments. The new test expects that file to bind, so the two agree.
  • Unit tests: the new cases cover plugins/review/ paths binding both lanes (including the bare directory) and a plugins/review/ change beside another lane source still unbinding.
  • Structure test: I traced scripts/ci-review-plugin-source.test.sh against the current callers. Each of pr-review-hosted.yml and pr-review-security-hosted.yml has exactly one pr-review-cli.yml call, pinned to the 40-hex SHA 4fbceb4b…. Their with: keys and values hold no plugin key or plugin/ or plugins/ path. The test should pass on both callers, and its negative fixtures cover the guarded shapes.
  • Remaining gap: whether the reusable stages the review plugin from the default branch's tip rests on the author's link into ci-workflows, which I did not read. A PR can only repoint a caller by changing .github/workflows/, which still unbinds, so the guarantee holds as long as that claim is true.
  • Closing issue: none. The PR cites an owner decision, so there are no acceptance criteria to check.
  • Changelog fragment: bump: patch fits a bug fix to binder behavior.
  • Manifests, paved paths, rules: no plugin manifest changed. I did not resolve the paved-path index or the .claude/rules/ files, so the review applies no criteria from them.
  • Cost: none. The change adds no billed resource, and review runs spend only prepaid credit or subscription usage.
  • Trusted paths: none of the changed files match the listed globs.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit a931c45 Oct 25, 2026
69 checks passed
@kyle-sexton
kyle-sexton deleted the fix/lane-source-review-plugin branch October 25, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant