Skip to content

Gate review flows behind explicit opt-in (skip in regular review sessions) - #238

Merged
alexeyzimarev merged 1 commit into
mainfrom
flows-guard-regular-session-reviews
Jul 2, 2026
Merged

alexeyzimarev merged 1 commit into
mainfrom
flows-guard-regular-session-reviews

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Problem

When the flows MCP tools are registered in a regular interactive Codex/Claude session, a plain review request ("review my PR", "code review", "review this spec") triggered the kcap-review-flows skill and pushed the agent to call start_review_flow — which launches a separate hosted reviewer through the daemon and drives a multi-round loop the user never asked for. Observed in the wild as Codex announcing "…using kcap-review-flows as the review workflow guide" for an ordinary review.

Fix

Gate the flow behind explicit opt-in, consistently across the three places an agent sees flows guidance:

  • kcap/skills/review-flows/SKILL.md — narrow the description to explicit-flow intent only ("start a review flow", "submit for review", "re-review after I address findings") and explicitly exclude ordinary review verbs; add a prominent "When NOT to use" guard (ordinary review → review directly, don't call the tools); retitle ## When to use → ## Choosing the flow kind so the spec-review/code-review split isn't misread as "any review → use the flow".
  • src/Capacitor.Cli/Commands/McpFlowsServer.cs — prepend the same gate to the start_review_flow tool description. This is the cross-agent lever: Codex reads tool descriptions from tools/list because config.toml carries no server description.
  • kcap/.mcp.json — same gate on the Claude Code kcap-flows server description.
  • kcap/.claude-plugin/plugin.json — bump 1.7.1 → 1.7.2 so marketplace installs refresh the new skill text (matching how Guard review-flows skill against hosted-reviewer misfire #235 handled it).

Net effect: a plain review request → the agent reviews directly and reports findings; the hosted-reviewer flow runs only when the user explicitly asks for a review flow / to submit for review.

Relationship to #235

Follow-up to #235, which guarded the opposite case (flows tools absent → you're likely the hosted reviewer, review directly). This PR covers tools present in a regular session where the user just wants a direct review.

Notes

  • No dedicated Linear/GitHub issue tracked for this; happy to link one if there is/should be.
  • Agent-facing guidance + version bump only — no user-facing CLI surface change, so no README sync required.
  • dotnet build clean (0 warnings); no tests assert on the touched descriptions.

🤖 Generated with Claude Code

…ions)

The flows guidance triggered on any review verb, so a plain "review my PR"
in a normal Codex/Claude session loaded the kcap-review-flows skill and
pushed the agent toward start_review_flow — spinning up a SEPARATE hosted
reviewer the user never asked for.

Gate the flow behind explicit opt-in, consistently across the three places
an agent sees flows guidance:

- kcap/skills/review-flows/SKILL.md: narrow the description to explicit-flow
  intent only ("start a review flow" / "submit for review" / "re-review"),
  add a "When NOT to use" guard (ordinary review → review directly), and
  retitle "When to use" → "Choosing the flow kind" so the spec/code split
  can't be misread as "any review → use the flow".
- McpFlowsServer.cs: prepend the same gate to the start_review_flow tool
  description (cross-agent lever; Codex reads tool descriptions from
  tools/list since config.toml carries no server description).
- kcap/.mcp.json: same gate on the Claude Code kcap-flows server description.
- Bump plugin.json 1.7.1 -> 1.7.2 so marketplace installs refresh the skill.

Follow-up to #235 (hosted-reviewer misfire guard), covering the opposite
case: tools present in a regular session but the user wants a direct review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Gate hosted review flows behind explicit user opt-in

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Gate review-flow guidance behind explicit user opt-in (avoid accidental hosted reviewer loops).
• Update skill, MCP server, and tool descriptions to exclude ordinary “review my PR” requests.
• Bump Claude plugin version so marketplace installs pick up the new guidance.
Diagram

graph TD
  A["Agent session"] --> B["Flows guidance text"] --> C{"Explicit flow opt-in?"}
  C -->|"No"| D["Review directly"]
  C -->|"Yes"| E["start_review_flow tool"] --> F["kcap daemon"] --> G["Hosted reviewer"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Enforce opt-in in the tool protocol (new arg / confirm step)
  • ➕ Hard guarantee: prevents accidental launches even if descriptions are ignored
  • ➕ Easier to validate in tests and in telemetry (arg present/absent)
  • ➖ Breaking/behavioral API change to start_review_flow (all clients must update)
  • ➖ Adds friction for intentional flow users (extra parameter/step)
2. Rename tools/server to make “hosted reviewer” unmistakable
  • ➕ Reduces ambiguity without relying on long descriptions
  • ➕ Improves discoverability and “cost” signaling in tool lists
  • ➖ Potentially breaking for existing configurations and tooling references
  • ➖ Requires coordinated rollout across docs/config/clients
3. Split the skill into two (ordinary review vs hosted flow)
  • ➕ Cleaner routing if the agent platform supports skill selection well
  • ➕ Keeps flow skill narrowly scoped and reduces keyword overlap
  • ➖ More moving parts and more skill maintenance
  • ➖ Doesn’t help if tool descriptions/config still steer incorrectly

Recommendation: The PR’s approach (tightening descriptions in SKILL.md, MCP server config, and the tool description) is the lowest-risk fix and targets the actual misfire mechanism: LLM routing based on available guidance/tool text. Consider a protocol-level opt-in only if misfires persist, since it would be more robust but also more disruptive.

Files changed (4) +25 / -11

Documentation (1) +20 / -8
SKILL.mdTighten skill trigger conditions and add “When NOT to use” guardrails +20/-8

Tighten skill trigger conditions and add “When NOT to use” guardrails

• Narrowly scopes the skill description to explicit review-flow intent and explicitly excludes ordinary review verbs. Adds a prominent section explaining that the tools hand off to a hosted reviewer, and retitles guidance to “Choosing the flow kind” to avoid implying all reviews should use flows.

kcap/skills/review-flows/SKILL.md

Other (3) +5 / -3
plugin.jsonBump Claude plugin version to 1.7.2 +1/-1

Bump Claude plugin version to 1.7.2

• Updates the plugin version so marketplace/installer refreshes pull the revised flows guidance text.

kcap/.claude-plugin/plugin.json

.mcp.jsonGate kcap-flows server description behind explicit opt-in language +1/-1

Gate kcap-flows server description behind explicit opt-in language

• Rewrites the 'kcap-flows' MCP server description to emphasize it launches a separate hosted reviewer and should only be used when the user explicitly requests a review flow. Explicitly instructs agents to review directly for ordinary review requests.

kcap/.mcp.json

McpFlowsServer.csAdd explicit opt-in gating to start_review_flow tool description +3/-1

Add explicit opt-in gating to start_review_flow tool description

• Updates the 'start_review_flow' tool description to state it hands work to a separate hosted reviewer and must not be used for ordinary review requests. This improves cross-agent routing because some clients primarily rely on 'tools/list' descriptions.

src/Capacitor.Cli/Commands/McpFlowsServer.cs

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@alexeyzimarev
alexeyzimarev merged commit c83c9d5 into main Jul 2, 2026
5 checks passed
@alexeyzimarev
alexeyzimarev deleted the flows-guard-regular-session-reviews branch July 2, 2026 16:42
realtonyyoung added a commit that referenced this pull request Jul 2, 2026
…us failure handling

Rebased onto current main (which #235/#238 rewrote for opt-in gating and the
hosted-reviewer misfire guard, and #233/#234 made hosting vendor-uniform).
Dropped my earlier "Roles" section (now covered by main's intro + "when NOT
to use" gating) and the Codex-specific wording (reviewer vendor now comes
from the flow definition, not hardcoded Codex).

Remaining unique, vendor-neutral addition: a Prerequisites section stating
start_review_flow needs `kcap login` + a daemon with this repo checked out,
and split failure handling — no-daemon (start one, retry) vs ambiguous
multi-checkout match (can't disambiguate; surface it, AI-1112).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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