Warn when a review flow's reviewer participant vendor disagrees with the applied echo

[realtonyyoung]

Nothing re-validated the reviewer's vendor after the start-time echo check (CheckVendorOverrideResult): a wrongly-vendored reviewer appearing later — the realistic path is a heal relaunch resolving a different vendor than the run pinned — would reach the driver silently, with status rendering both fields side by side and never noticing they disagree.

This is the CLI (defensive) half of the cross-check; the server (authoritative) half is kurrent-io/kcap-server#1225. Either half works without the other.

Linear: AI-1585 (closes AI-1489 §8's last uncovered negative case).

What

AppendReviewerVendorMismatchWarning cross-checks applied_reviewer_vendor against the reviewer participant's vendor from the same response body, and additionally honors the server's new reviewer_vendor_mismatch flag. On disagreement it renders:

⚠ reviewer vendor mismatch: participant 'reviewer' is 'claude' but applied_reviewer_vendor is 'codex'.
  A different reviewer than this run reported may be doing the work — treat its results as suspect: close the flow and report this.

on both surfaces that parse the status body:

• FormatStatusResponse (get_flow_status / get_review_flow_status) — after the participants block it contradicts;
• FormatPolledRoundResult (the async round-result poll) — before the result text, so the driver reads the warning before the suspect result.

Decisions

• Local comparison first, server flag as the fallback. The local comparison works against servers that predate the server-side check; the flag covers a response whose participant list is read-model-lagged. Either alone triggers the warning.
• Warning, not a tool error. These responses carry the round results and pending messages the driver still needs — erroring the poll would derail the loop without delivering the information. The start-time echo check keeps its hard-error + defensive close: at start the CLI knows the requested vendor with certainty; at status it only sees two server fields disagreeing, and the driver decides.
• Reviewer-scoped. The run-level echo describes only the reviewer role; a multi-participant flow's other roles legitimately run different vendors and never trip the warning. An absent vendor in a partial body is not evidence of disagreement.

Tests

Mismatch, server-flag-only (no participants), agreement negative, non-reviewer-role negative, and polled-surface ordering. Mutation-tested: dropping the local comparison fails the two local-arm tests; dropping the flag arm fails the flag test.

🤖 Generated with Claude Code

[linear-code]

AI-1489

AI-1585

[qodo-code-review]

PR Summary by Qodo

Warn when reviewer vendor mismatches applied reviewer vendor in CLI output

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add a CLI warning when reviewer participant vendor disagrees with applied reviewer vendor.
• Also honor server-provided reviewer_vendor_mismatch when participants are missing or stale.
• Add unit tests covering mismatch, flag-only warning, role scoping, and warning ordering.

Diagram

graph TD
  S["Flow status/round JSON"] --> FS["FormatStatusResponse"] --> W["AppendReviewerVendorMismatchWarning"] --> O["CLI formatted output"]
  S --> FP["FormatPolledRoundResult"] --> W

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Rely solely on server reviewer_vendor_mismatch flag

• ➕ Centralizes mismatch detection on the authoritative server
• ➕ Avoids client-side parsing/role matching logic
• ➖ Breaks/weakens detection against older servers
• ➖ Doesn’t catch mismatches if the flag is missing or delayed

2. Treat mismatch as a hard CLI error (fail the tool call)

• ➕ Forces immediate attention and prevents drivers from consuming suspect results
• ➖ Interrupts polling loops and may hide pending messages/round results the driver still needs
• ➖ Can create operational friction if mismatches are transient/read-model artifacts

3. Proactively close the flow from the CLI when mismatch is detected

• ➕ Reduces chance of continued work under a mis-pinned reviewer vendor
• ➖ Client-side auto-closure can be surprising and may conflict with server-side policy/auditing
• ➖ Requires additional error handling and retries around close semantics

Recommendation: Keep the current approach: perform a local same-body comparison for backward compatibility and add the server flag as a fallback for read-model lag, while rendering a warning (not an error) so the driver still receives round results and pending messages. This balances safety with operational continuity and matches the CLI’s limited certainty at status-time.

Files changed (2) +107 / -0

Bug fix (1) +42 / -0

McpFlowsServer.csRender reviewer-vendor mismatch warning in status and polled round output +42/-0

Render reviewer-vendor mismatch warning in status and polled round output

• Adds AppendReviewerVendorMismatchWarning to detect disagreement between applied_reviewer_vendor and the reviewer participant’s vendor in the same response body, or via reviewer_vendor_mismatch. Invokes the warning from FormatStatusResponse (after participants) and from FormatPolledRoundResult before round result text so drivers see the warning first.

src/Capacitor.Cli/Commands/McpFlowsServer.cs

Tests (1) +65 / -0

McpFlowsServerTests.csAdd unit coverage for reviewer vendor mismatch warnings and ordering +65/-0

Add unit coverage for reviewer vendor mismatch warnings and ordering

• Adds tests asserting the warning appears on vendor disagreement, on server-flag-only responses, and never for agreement or non-reviewer role differences. Verifies the warning is rendered before round result text in the polled surface.

test/Capacitor.Cli.Tests.Unit/McpFlowsServerTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. README missing vendor mismatch warning ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The CLI now renders a new user-facing ⚠ reviewer vendor mismatch warning in flow status / poll
output, but the PR does not update README.md to document this behavior. This violates the
requirement to update README in the same PR for user-facing CLI surface changes.

Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R1266-1272]

+        sb.AppendLine();
+        sb.Append("⚠ reviewer vendor mismatch: ");
+        sb.AppendLine(participantVendor is not null
+            ? $"participant 'reviewer' is '{participantVendor}' but applied_reviewer_vendor is '{applied}'."
+            : "the server flagged that the active reviewer's vendor disagrees with applied_reviewer_vendor.");
+        sb.AppendLine("  A different reviewer than this run reported may be doing the work — treat its " +
+                      "results as suspect: close the flow and report this.");

Evidence

McpFlowsServer now appends a new ⚠ reviewer vendor mismatch warning to the formatted output for
polled round results and flow status, which is user-facing CLI behavior. The README’s Flows MCP
server section documents get_flow_status and other warnings but does not mention this new
vendor-mismatch warning, so the README was not updated alongside the behavior change.

CLAUDE.md: README.md Must Be Updated in the Same PR for Any User-Facing CLI Surface Change
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1109-1113]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1209-1211]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1266-1272]
README.md[408-434]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CLI now emits a new user-visible warning (`⚠ reviewer vendor mismatch`) on flow status and polled round-result surfaces, but `README.md` was not updated to mention it.

## Issue Context
The compliance checklist requires README updates in the same PR for any user-facing CLI surface change. This PR changes the rendered output of the flows status tools by adding a new warning block.

## Fix Focus Areas
- README.md[408-446]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1109-1113]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1209-1211]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1266-1272]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Stopped reviewer false warning ✓ Resolved 🐞 Bug ≡ Correctness

Description

AppendReviewerVendorMismatchWarning warns on any reviewer participant vendor mismatch without
checking the participant's stopped status. If the participants list includes a previously-stopped
reviewer with a different vendor, the CLI can emit a mismatch warning even when the active reviewer
matches applied_reviewer_vendor.

Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R1248-1259]

+        string? participantVendor = null;
+        if (applied is not null && node["participants"] is JsonArray participants)
+            foreach (var item in participants) {
+                if (item is not JsonObject p) continue;
+                if (!string.Equals(StringField(p, "role"), "reviewer", StringComparison.Ordinal)) continue;
+                var vendor = StringField(p, "vendor");
+                // An absent vendor in a partial body is not evidence of disagreement.
+                if (vendor.Length > 0 && !string.Equals(vendor, applied, StringComparison.Ordinal)) {
+                    participantVendor = vendor;
+                    break;
+                }
+            }

Evidence

The file already models participant activity via a stopped boolean (rendered as running/stopped),
but the new mismatch scan ignores stopped and will warn on any reviewer vendor mismatch it
encounters.

src/Capacitor.Cli/Commands/McpFlowsServer.cs[1220-1233]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1245-1259]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AppendReviewerVendorMismatchWarning` treats any participant with `role == "reviewer"` as authoritative and will warn if its `vendor` differs from `applied_reviewer_vendor`. However, the participant model includes a `stopped` field (and the CLI already renders it), implying the participant list can contain inactive/historical entries; those should not trigger a mismatch warning.

### Issue Context
This warning is operationally consequential (it tells the driver results are suspect and to close/report). A stopped historical reviewer from an earlier lifecycle event (relaunch/rotation/heal) could therefore create a false alarm.

### Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1245-1273]

### Implementation notes
- When scanning participants, skip reviewer entries where `stopped == true` (same bool parsing pattern as `AppendParticipants`).
- If there can be multiple reviewer entries, consider preferring the first non-stopped reviewer (or last non-stopped reviewer, depending on server ordering contract).
- Add a unit test where participants include:
 - a stopped reviewer with vendor != applied
 - an active reviewer with vendor == applied
 and assert **no** mismatch warning is rendered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Overlong AppendReviewerVendorMismatchWarning comment ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new XML doc comment for AppendReviewerVendorMismatchWarning is unusually verbose and restates
extended rationale that could be conveyed more concisely. This reduces readability and violates the
guideline to keep comments concise.

Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R1236-1244]

+    /// <summary>Defensive cross-check of the run-level applied reviewer vendor against the
+    /// "reviewer" participant's vendor in the SAME response body — nothing re-validates the vendor
+    /// after the start-time echo check, so a wrongly-vendored reviewer (e.g. a heal relaunch that
+    /// resolved a different vendor than the run pinned) would otherwise reach the driver silently.
+    /// The local comparison also works against servers that predate the server-side check; the
+    /// server's <c>reviewer_vendor_mismatch</c> flag covers a response whose participant list is
+    /// read-model-lagged. Renders a warning instead of erroring the tool call: these responses
+    /// carry the round results and pending messages the driver still needs, and the driver decides
+    /// whether to close.</summary>

Evidence

The comment block added above AppendReviewerVendorMismatchWarning spans many lines and includes
detailed narrative explanation (examples and multi-sentence rationale), which exceeds what’s
typically needed to understand the code at point-of-use. This matches the checklist’s failure
criteria of introducing unnecessarily verbose comments.

CLAUDE.md: Keep Comments Concise; Prefer Self-Explanatory Code Over Verbose Commentary
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1236-1244]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The XML doc comment introduced for `AppendReviewerVendorMismatchWarning` is long and detailed, increasing noise in the code.

## Issue Context
The repo’s compliance checklist asks to keep comments concise and prefer self-explanatory code. The function name and the emitted warning already explain most of the behavior; only a brief rationale and key semantics are needed.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1236-1244]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}