Make cross-tenant review findings always Red (CROW-1321)

[dhilgaertner]

Closes #1321

Part of epic corveil/corveil#4203 (wave 1). Incident: corveil/corveil#4191.

Problem

Two rules in crow-review-pr graded a real cross-tenant bug Green. In corveil/corveil#620 the review said "Org A's keyword/regex rules apply to Org B's traffic". It capped that at Green under the accepted-risk rule and approved. The bug later blocked every tenant's gateway traffic.

Change

This is a prompt-only change. I extended the existing {{CROW_REVIEW_GRADING_GUIDANCE}} block instead of adding a placeholder. The floor is now the first rule in that block, so it always comes before the caps. It also reaches every harness through the path that already exists: the copied SKILL.md for Claude, and the inlined prompt for Cursor, Codex, OpenCode, Grok, Antigravity and Muse.

ReviewVerdictPolicy.gradingGuidanceBlock now renders, in this order:

• The floor. Cross-tenant effect means one tenant's config or data changes another tenant's requests, responses, records, limits, availability or costs. Cross-tenant exposure means one tenant can read another tenant's config or data. A finding of either is always Red. The floor says outright that the accepted-risk, declined-finding and grade-against-the-diff rules don't apply to it. Each of those three rules also carries the exemption on its own line, because the docs: drop optional mise task-runner dependency #620 reviewer applied the accepted-risk rule by itself.
• The deferral bar. A cross-tenant finding can merge as deferred only when the reviewer has checked each of these:
  • an open tracking issue (gh issue view <N> --json state), and not one this PR closes;
  • a // DEFERRED(#N): marker at the site;
  • an exceptions-table row in the repo's tenant-isolation ADR, where the repo has one.
• The tenant-effect checklist. These are the ticket's four questions. "Yes" to Q2 is a cross-tenant finding. "Yes" to Q3 is one too, unless the reviewer cites code that rejects a missing org or fails closed. The checklist is n/a — <why> when the repo has no tenancy or the diff touches no tenant-scoped code. Repo rules from CLAUDE.md or an ADR can add to the floor but never lower it.

skills/crow-review-pr/SKILL.md (both halves, byte-identical) changes in three places:

• The review-body format gets a ### Tenant effect section, required in every review.
• Step 5a gets a third check, Tenant effect check. It refuses a draft that has no section, uses n/a while citing tenant-scoped code, or answers "yes" to "Can org A's row change org B's outcome?" without a Red finding or a fully checked Deferred (#N) entry. This is the semi-mechanical verdict check the ticket asks for.
• Step 4b closes another way a cross-tenant Red could be lowered. An unverified Red is downgraded to Yellow, so a reviewer who couldn't reproduce a cross-tenant bug could have downgraded it. Now a cross-tenant finding is refute-checked against the code that would scope it to its tenant. If no such code exists, that absence counts as the verification.

scripts/check-workspace-custom-instructions.sh now requires the new static prose in both halves.

The text follows invariant 10 of the ADR 0093 draft (corveil/corveil#4204) but stays generic. Corveil-specific rules arrive through corveil's own CLAUDE.md (corveil/corveil#4215).

Tests

• ReviewVerdictPolicyTests (CrowCore): 27 tests pass, 6 of them new.
  • The floor text and its definitions are present.
  • The floor comes before each of the three caps.
  • Each cap's own line exempts cross-tenant findings.
  • The deferral bar is present, including "an issue this PR closes does not count".
  • The four checklist questions are present.
  • On the real skill, under the default, red-only and block-everything policies: the floor comes before the accepted-risk rule, and the ### Tenant effect section, the Step 5a check and the Step 4b no-downgrade sentence are all present.
• SessionServiceReviewPromptTests (CrowEngine): 21 tests pass, 1 of them new. For all six inline harnesses, under both the default and red-only policies, the built prompt carries the floor before the accepted-risk rule. The accepted-risk line exempts cross-tenant findings, and the deferral bar and checklist are present.
• scripts/check-workspace-custom-instructions.sh passes.

Not done here

• Regenerating the installed skill. Crow installs crow-review-pr from the repo checkout of the running crowd, both into {devRoot}/.claude/skills/ and into each review clone. I didn't overwrite the installed copy from an unmerged branch, because every live review in the dev root reads it. After merge, rebuild and restart crowd. The next review session will then show the floor, the ### Tenant effect section and the Step 5a check.
• Gating is unchanged. The floor sets the grade. Whether Red blocks is still the workspace's reviewBlockingSeverities, which is Red in both the default and red-only policies.

Sibling overlap

• corveil/corveil#4205 (deferral integrity) enforces the DEFERRED(#N) → open-issue rule in corveil's CI. This PR makes the reviewer check the same thing.
• corveil/corveil#4213 adds a matching "Tenant effect" section to corveil's PR template, for authors.

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Critical Issues

None.

Architecture / Existing Patterns

• Existing pathway: ReviewVerdictPolicy.gradingGuidanceBlock, substituted for {{CROW_REVIEW_GRADING_GUIDANCE}} on every harness. The review-body ### Tenant effect section, the Step 5a check, and the Step 4b sentence are static skill prose, the same shape as the CROW-974 guardrails and the CROW-1062 architecture study. This change extends that path.

The floor, deferral bar, and checklist sit in the policy block, so Claude's copied skill and the inlined prompts receive one text. Both skill halves match, and scripts/check-workspace-custom-instructions.sh passed.

Tenant effect

n/a — the diff is review-rubric prose and string-pin tests. It does not touch tenant-keyed rows, process-wide state built from them, or a function that takes a tenant argument.

Security Review

Strengths:

• The cross-tenant floor is the first grading rule and defines effect and exposure in the ticket's terms (Packages/CrowCore/Sources/CrowCore/ReviewVerdictPolicy.swift).
• Each cap's opening clause exempts a cross-tenant finding. The deferral bar requires an open issue this PR does not close, a DEFERRED(#N) marker, and an ADR exception row where the repo has one.
• Step 4b treats missing tenant-scoping code as the verification, so an unverified cross-tenant Red stays Red.

Concerns:

• Yellow — the declined-finding bullet still ends by instructing Green. See Code Quality.

Code Quality

• Yellow — the declined-finding line still says "leave it Green." Confirmed by reading Packages/CrowCore/Sources/CrowCore/ReviewVerdictPolicy.swift:224. The floor says that rule does not apply to a cross-tenant finding, and the first clause now says the finding stays Red until it is fixed or clears the deferral bar. The same line then continues, unqualified: "Re-raising the same point at a blocking severity round after round is not permitted; if you remain unconvinced, leave it Green and, if it is worth tracking, file a follow-up issue instead of holding up the PR." That second sentence is the round-2 case this rule exists for — the author answered with a rationale and the reviewer is unconvinced. Read on its own terms, which is the #620 failure mode and the reason everyCapExemptsCrossTenantFindings pins the carve-out on the rule's own line, it tells the reviewer to stand down and grade Green. The pin at Packages/CrowCore/Tests/CrowCoreTests/ReviewVerdictPolicyTests.swift:361-362 only requires unless it is a cross-tenant finding, which stays **Red** somewhere on that line, so the trailing instruction still passes. Qualify the trailing sentence the way the grade-against-the-diff bullet qualifies its Green cap (ReviewVerdictPolicy.swift:225): a cross-tenant finding stays Red until it is fixed or clears the deferral bar. The accepted-risk bullet's second sentence (ReviewVerdictPolicy.swift:223) has the same shape — "let the verdict follow the grade, rather than blocking on it" — and should say the verdict follows the Red floor when the finding is cross-tenant.
• scripts/check-workspace-custom-instructions.sh passed. swift test for ReviewVerdictPolicyTests and SessionServiceReviewPromptTests did not run: compilation stopped because the volume was out of space. The pins were checked by reading the block against the assertions.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

Recommendation: Request Changes — driven by [0 Red, 1 Yellow, 0 Green] findings.

---

🐦‍⬛ Reviewed by Crow via Cursor

[dgershman]

Code & Security Review

Critical Issues

None.

Architecture / Existing Patterns

• Existing pathway: ReviewVerdictPolicy.gradingGuidanceBlock, substituted for {{CROW_REVIEW_GRADING_GUIDANCE}} by ReviewVerdictPolicy.expand and then handed to both consumers in prepareReviewClone: the copied SKILL.md and buildReviewPrompt. The ### Tenant effect section and the Step 5a check are static skill prose, the same shape as the CROW-974 guardrails. This change extends that path.

The floor, deferral bar, and checklist live in the policy block, so every harness gets one text. Both skill halves match. The follow-up on this PR qualifies each stand-down sentence in the three caps, and everyStandDownClauseInACapCarvesOutCrossTenantFindings passes.

Security Review

Strengths:

• The cross-tenant floor is the first grading rule. It defines effect and exposure, and it says the accepted-risk, declined-finding, and grade-against-the-diff rules do not apply (Packages/CrowCore/Sources/CrowCore/ReviewVerdictPolicy.swift).
• Each cap's own stand-down sentences carve a cross-tenant finding back to Red, including "leave it Green" and "rather than blocking."
• The deferral bar requires an open issue this PR does not close, a // DEFERRED(#N): marker, and an exceptions-table row where the repo has a tenant-isolation ADR. A missing item keeps the finding Red.
• Step 4b treats missing tenant-scoping code as the verification, so an unverified cross-tenant Red stays Red (skills/crow-review-pr/SKILL.md).
• Step 5a refuses a draft with no ### Tenant effect section, or a "yes" to "Can org A's row change org B's outcome?" without a Red finding or a checked Deferred (#N) entry.

Concerns:

• Green — Step 5a mechanically gates question 2, not question 3. The checklist says a "yes" to "Is any org argument dropped, Nil, or optional?" is a cross-tenant finding unless the reviewer cites code that rejects the missing org or fails closed (Packages/CrowCore/Sources/CrowCore/ReviewVerdictPolicy.swift). The tenant-effect check keys off question 2 and the words "cross-tenant finding" (skills/crow-review-pr/SKILL.md). A draft can answer question 3 "yes" without that citation and without labeling it a cross-tenant finding, and the check still passes. The ticket asked the verdict check to confirm the section is present; question 3 is the extra case left to the prose.

Code Quality

• ReviewVerdictPolicyTests (28) and SessionServiceReviewPromptTests (21) passed. scripts/check-workspace-custom-instructions.sh passed.
• The round-1 stand-down wording is fixed: a finding that is not cross-tenant is the only one those sentences tell the reviewer to leave Green.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

Recommendation: Approve — driven by [0 Red, 0 Yellow, 1 Green] findings.

---

🐦‍⬛ Reviewed by Crow via Cursor