feat(review): scope REVIEW.md per lane and re-derive the review credential

[kyle-sexton]

What

• REVIEW.md: restructured into mutually exclusive lane-scope sections — ## Code-review lane scope (audit-logging + atomicity always-checks) and ## Security lane scope (the four blocking always-checks). Deleted the report-stage filtering sections (## Do not report, ## Cap the nits) per Anthropic's Sonnet-5 harness-effect guidance. ## Severity, ## Depth, and blocking tag semantics unchanged.
• distribution/REVIEW-CREDENTIAL.md: re-derivation Re-derive REVIEW-CREDENTIAL classification: stale repo-visibility premise + add-dir visibility blocker now cleared #264 asked for, on top of docs(distribution): correct the review-credential visibility facts #276's corrected visibility facts — with standards public no consumer needs the credential (anonymous checkout reaches the same tree); the --add-dir subagent-visibility open question marked empirically resolved; provisioning stays deferred behind github-iac governance.

Closes #264

Related

• docs(distribution): correct the review-credential visibility facts #276 — corrected the visibility facts this re-derivation builds on (merged 2026-07-26).
• melodic-software/ci-workflows docs/topics/claude-review-lanes/PLAN.md — the approved implementation plan; this PR is its Phase 1. Phase 2's model-switch PR is gated on the five downstream review-instructions sync PRs this merge triggers.
• Wave-2 standards grounding rollout inventory (user-reserved scope decision) claude-code-plugins#289 — carries the verification annex Re-derive REVIEW-CREDENTIAL classification: stale repo-visibility premise + add-dir visibility blocker now cleared #264 drew from.

Notes

• Independent verification (fresh-context agent, 7/7 criteria PASS + 1 blocking flag, fixed in 50b4969): the first cut asserted the security lane consumes this file (it runs from its own security-only prompt and never reads it) and made the security-findings exclusion unconditional — which would have suppressed the four blocking checks on the five repos whose only reviewer is the code-review lane until the Phase 3 fleet rollout. The intro now records scope ownership rather than wiring, and the exclusion is gated on a claude-security-review workflow existing in the repo — self-resolving as the rollout proceeds, no coverage window.
• Deviation from the plan's original premise: the plan predates docs(distribution): correct the review-credential visibility facts #276 and framed the re-derivation as "claude-code-plugins is public → ineligible". docs(distribution): correct the review-credential visibility facts #276 established standards itself is public, which supersedes per-repo eligibility — the re-derivation records that no credential is needed at all under current state. Direction (defer provisioning) unchanged.
• Deviation: the plan said "update ## Review triggers" in REVIEW-CREDENTIAL.md; left untouched — verified the existing triggers already cover both credential-return cases (visibility change; open-question resolution).
• Known loss: the deleted "Do not report" section was the only REVIEW.md mention of conventions/review/overlays/; overlays still reach plugin surfaces via conventions/review/README.md.
• Follow-up (ci-workflows Phase 2): claude-review.yml's default prompt says "review for … security"; with this split that wording should name the lane and defer security scope per this file.
• review-instructions is a managed component in 5 repos and locally-owned in medley; medley content-equivalence is a Phase 5 follow-up in the plan above.
• Cross-doc reconciliation swept: no other doc cites the deleted sections; conventions/review/README.md's blocking crosswalk statements still hold against the restructured file.

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 3m 19s —— View job

---

Code Review — PR #278

• Gather diff and context
• Read changed files in full
• Read REVIEW.md criteria and cross-referenced conventions
• Post review findings

---

REVIEW.md

Lane introduction (lines 10–14) — correct and precisely worded. The mutual-exclusion assertion gives agents a clear decision rule, and naming both lane identifiers (claude-review / claude-security-review) makes it resolvable without runtime context injection. No issue.

Code-review lane scope (lines 57–77) — the two audit-log and atomicity checks are correctly placed here. The disambiguation sentence on lines 70–72 ("This is an observability completeness check on the logging seam; whether the action itself is safe is the security lane's question") is the key that prevents double-reporting of security-adjacent heuristics, and it's worded well.

Security lane scope (lines 79–100) — all four blocking items are unchanged. One minor wording note:

This lane reports only security findings: vulnerabilities, missing authorization or tenant scoping, exposed secrets or credentials, injection, and their direct enablers.

"Direct enablers" is introduced here without definition, which could pull in design or test-coverage findings if an agent reads it broadly. The four always-checks bound it implicitly, but adding one concrete example (e.g., "a missing validation that makes injection reachable") or replacing "direct enablers" with a tighter term ("or code paths that immediately allow one of the above") would remove the interpretive gap. Not a blocker, but worth a pass before this propagates to managed consumers.

Deleted ## Do not report — this is the most behaviorally significant change. Without this section, the code-review lane now has no explicit suppressor for:

1. CI-enforced items (lint, formatting, type errors) that the overlay backstop mechanism already catches.
2. Non-blocking noise in generated files and lockfiles.

The PR description cites "Anthropic's Sonnet-5 harness-effect guidance" as the rationale, but that citation isn't linked and doesn't appear in the file or the commit message. If the guidance is documented somewhere (a Slack thread, an internal doc, an issue), linking it — even in a commit note — would let a future editor know why the suppressors were removed rather than concluding they were accidentally dropped. If the Sonnet-5 behavior simply makes these instructions redundant, that's worth one sentence inline.

Deleted ## Cap the nits — same question: the rationale is in the PR description but not the file. If downstream consumers relied on the five-nit cap as a cost-control mechanism, the silent removal could cause token budget surprises on verbose diffs. If Sonnet-5 self-regulates well enough to make the cap unnecessary, that claim is worth stating in the file rather than only in the PR.

Cross-doc reconciliation claim — verified. No file in the repo references the deleted section headers (## Do not report, ## Cap the nits, or their content). The conventions/review/README.md blocking crosswalk statement ("REVIEW.md's crosswalk says so plainly rather than implying parity that doesn't exist") still holds against the restructured file; the Severity section and its blocking explanation are unchanged.

---

distribution/REVIEW-CREDENTIAL.md

Status line update (lines 15–20) — the amended wording ("re-derived 2026-07-26 as not currently needed") is accurate given the re-derivation section that follows it. ✓

Re-derivation section (lines 101–122) — the three-bullet summary is logically correct:

• "No consumer needs this credential today" follows directly from standards being public.
• "Provisioning stays deferred" is the right posture given the no-op result.
• "The need returns in exactly two cases" is precise: private reclassification of a mounted source, or a documented private-marketplace auth requirement. The existing Review triggers section (lines 240–246) is the broader recheck mechanism, so these two cases correctly describe when a credential would be needed, not when to re-run the classification.

--add-dir visibility resolution (lines 216–222) — empirical resolution with a link to #264 is the right way to close this. The description of what was tested (Read, Glob, Grep against a directory outside the primary working directory) matches the mechanism --add-dir uses, so the resolution is supported. ✓

Retained ## Classification section — the CAUTION block on line 25 ("This section's premise no longer holds. Do not provision from it.") is prominent enough to prevent accidental use of the superseded scope list. Leaving the section intact for re-derivation reference is the correct call given the documented rationale.

---

Summary

The lane-scoping restructure is logically sound and the mutual-exclusion design is correctly implemented. The credential re-derivation is accurate and well-documented. Two items worth a follow-up before this propagates to managed consumers:

1. "Direct enablers" wording in the security lane intro — tighten or example-annotate to avoid over-reporting.
2. In-file rationale for the deleted ## Do not report and ## Cap the nits sections — one sentence each (or a linked citation) so future editors know the removals were intentional Sonnet-5 behavior decisions rather than accidental.

Neither is a blocker for merge given the cross-doc sweep confirmed no dangling references, but both will matter when the managed consumers receive the update.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: edfdf10f5b

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 3s —— View job

---

Code Review — PR #278

• Gather diff and read changed files
• Review REVIEW.md changes
• Review distribution/REVIEW-CREDENTIAL.md changes
• Cross-check against conventions and prior review findings
• Post final review

---

Review

This is the second pass, covering HEAD (50b4969) after the fix commit addressed two defects caught in the first round. The prior review's open items and the two Codex P2 findings are assessed below.

---

What the fix commit correctly resolved

Commit 50b4969 ("fix(review): state the lane split as intent, gate exclusion on lane presence") addressed two real defects in edfdf10:

1. Security lane reads this file — false. The original intro said "Two review lanes consume this file," which was incorrect: the security lane runs from its own security-only prompt and never reads REVIEW.md. The fix rewrites the intro so the security lane's scope section is described as being there for surfaces other than the security lane (the code-review lane knowing what to leave aside, and a self-hosted security agent applying the right section). This is accurate.

2. Unconditional security-findings exclusion — coverage gap. The original code-review lane scope said "It does not report security findings" with no qualifier, which would silently drop the four blocking checks on any repo not yet running claude-security-review. The fix correctly gates the exclusion on the presence of a .github/workflows/claude-security-review.yml workflow. The fallback ("report security findings under this lane too, applying the security-scope checks below") closes the coverage window. Both the conditional and the fallback are precisely worded.

---

REVIEW.md — remaining items

"Direct enablers" — wording gap, not a blocker (line 92)

This lane reports only security findings: vulnerabilities, missing authorization or tenant scoping, exposed secrets or credentials, injection, and their direct enablers.

This phrase was raised in the prior review and is unchanged. "Direct enablers" has no definition in the file or in any cited criterion. The four always-checks constrain it implicitly, but a reviewing agent reading the intro in isolation could pull in test-coverage gaps or design findings on the theory that they "enable" a vulnerability. A one-word tightening — "and code paths that immediately allow one of the above" — or removing the phrase and relying on the always-checks to bound scope would close this. Not a blocker, but worth fixing before this propagates to managed consumers. Fix this →

Deleted sections — no in-file rationale (formerly lines 110–126 of origin/main)

The ## Do not report and ## Cap the nits sections were removed. The commit message explains why ("Anthropic's Sonnet-5 harness-effect guidance"), but that rationale is absent from the file. Managed consumers receive only the file content — not the commit history — so a future editor or managed agent has no in-file signal that these sections were deliberately removed versus accidentally dropped. A single sentence at the end of the intro or in a comment (if this file permits comments) — "The Sonnet-5 harness self-regulates noise and nit volume sufficiently that explicit caps are no longer necessary" — would preserve the intent without adding bulk. As raised in the prior review, if the guidance is documented externally, linking the reference is the minimum; without it this rationale is unverifiable.

---

distribution/REVIEW-CREDENTIAL.md — Codex P2 findings assessed

Codex P2 at line 113 — "Reconcile the public-caller prohibition"

The Codex finding notes that line 89 unconditionally forbids the native-reference mount on public calling repos, while the re-derivation section says that prohibition is moot. This tension is real, but it is already handled: line 89 sits inside the ## Why a dedicated credential section, which opens with a prominent CAUTION block (lines 24–25) stating "This section's premise no longer holds. Do not provision from it." The prohibition at line 89 is clearly framed as superseded. A reader who reads the re-derivation section first without hitting the CAUTION could be confused, but the document's ordering (CAUTION appears before line 89, re-derivation section after) and the CAUTION's explicit "do not provision from it" instruction adequately guard against acting on the stale prohibition. No change strictly required; cross-referencing the CAUTION from the re-derivation section would make navigation easier.

Codex P2 at line 117 — "Do not reuse the superseded credential classification"

The classification below is retained as the record of what would be provisioned if the need returns.

The Codex finding points at a tension: the re-derivation says the classification records "what would be provisioned," while the CAUTION at lines 83–87 says "Re-derive the requirement from scratch before anything is created." These two claims are consistent in intent (the CAUTION wins at provisioning time), but the re-derivation's wording — "what would be provisioned" — can be read as implying the retained classification is the right starting point rather than a historical artifact. Changing "what would be provisioned" to "what was proposed" or "the superseded proposed scope" removes that ambiguity and aligns with the CAUTION's explicit instruction to re-derive from scratch. Fix this →

--add-dir visibility resolution (lines 216–222) — no issue. Empirical resolution with an issue link and the specific tools tested (Read, Glob, Grep against an out-of-primary-directory path) is the correct way to close this. ✓

---

Summary

The second commit correctly addresses both defects caught in the initial review. The restructure's mutual-exclusion design and the credential re-derivation are sound. Three items worth resolving before this propagates to managed consumers:

Item	File	Severity
"Direct enablers" undefined — could pull in out-of-scope findings	REVIEW.md:92	Nit
No in-file rationale for deleted ## Do not report / ## Cap the nits	REVIEW.md	Nit
"What would be provisioned" implies retained classification is usable, contradicting the CAUTION's "re-derive from scratch"	REVIEW-CREDENTIAL.md:117	Nit

None are blockers given the CAUTION already guards against misuse of the superseded classification, and the deleted sections have no dangling cross-references. The cross-doc reconciliation claim from the prior review stands — verified in the first pass, no new references introduced by either commit.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50b49697d4

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 48s —— View job

---

Code Review — PR #278 (commit 18bda17)

• Gather diff and understand what changed in the latest commit
• Read changed files in full
• Assess whether the new commit resolves the outstanding Codex P2 finding on composition
• Cross-check credential doc changes against the CAUTION / re-derivation section
• Post final review findings

---

This is the third pass, covering commit 18bda17 ("docs(review): reconcile public-standards state across credential and composition docs"). Three Codex P2 findings and one prior-review nit remained open after 50b4969; all three P2s are resolved here.

---

What this commit correctly resolved

Codex P2 — Composition standard vs two-lane policy (ai-review-bot-composition.md:9)

The old text said "One PR-time AI reviewer" for the Governed class, directly contradicting REVIEW.md's declaration of two mutually exclusive lanes. The fix correctly reframes the baseline as "Two PR-time AI lanes, split by scope rather than doubled up" and adds the critical disambiguation sentence:

The security lane is scope partitioning, not the same-scope second reviewer whose evidence-gated exception defines the high-traffic class below.

That sentence does the necessary work: it explains why adding the security lane doesn't trigger the evidence-gated exception, and it cross-references REVIEW.md as the authoritative split record. The "Why the class" section's update from "single-reviewer default" to "default lane composition" is consistent. ✓

Codex P2 — Public-caller prohibition overclaiming (REVIEW-CREDENTIAL.md:89)

The old unconditional "The native-reference mount is forbidden on any public calling repo" was rewritten to target private content specifically, with explicit confirmation that mounting public content is permitted. This is internally consistent with:

• The re-derivation section (no credential needed because standards is public)
• conventions/README.md's updated consumer-scope language (now "A consuming repo's REVIEW.md can cite..." rather than "A private consuming repo's")

The three documents now tell a consistent story. ✓

Codex P2 — "What would be provisioned" contradicts the CAUTION (REVIEW-CREDENTIAL.md:118)

Replaced with "retained strictly as the historical record of the superseded proposal" plus explicit instruction to re-derive from scratch and a note that the marketplace-auth case may require a different credential shape. This removes the implication that the retained classification is a usable starting point, aligning with the CAUTION's "do not provision from it." ✓

---

Remaining open items (carry-forward from second review pass)

Item	File	Severity
"Direct enablers" undefined — could pull in out-of-scope findings	REVIEW.md:92	Nit
No in-file rationale for deleted ## Do not report / ## Cap the nits	REVIEW.md	Nit

Both were raised in the first and second review passes and are unchanged. Neither is a blocker — the cross-doc sweep confirmed no dangling references to the deleted sections, and the "direct enablers" phrase is implicitly bounded by the four always-checks. Both will matter when managed consumers receive the sync PR.

---

Summary

All three Codex P2 findings are correctly resolved by this commit. The lane-scoping restructure, the credential re-derivation, and the public-content mount permission are now internally consistent across REVIEW.md, conventions/README.md, conventions/review/ai-review-bot-composition.md, and distribution/REVIEW-CREDENTIAL.md. The two remaining nits are pre-existing carry-forwards; no new issues introduced by this commit. Ready to merge on those terms.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18bda17a57

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Gate security suppression on a lane that can run

When a repository first adds .github/workflows/claude-security-review.yml, the file exists in the PR checkout and therefore satisfies this test, but a pull_request workflow newly introduced by that PR cannot run until the workflow exists on the default branch. The code-review lane will consequently omit every security finding from the rollout PR even though no security lane reviews it, so any security-sensitive workflow change bundled into that PR has no owning reviewer. Gate suppression on the security lane being active for the current PR (or on the caller existing in the base branch), rather than mere head-tree file existence.

AGENTS.md reference: AGENTS.md:L29-L32

Useful? React with 👍 / 👎.