[AI-1677] flows.reviewer_vendor preference + vendor-less definition fallback

README.md

@@ -424,7 +424,7 @@ Stdio MCP server that lets coding agents start and interact with AI-powered agen
 
 It provides four generic tools:
 
-- **`start_flow`** — start a new flow from either `definition_id` (the server's flow-definition catalog, e.g. `spec-review`, `code-review`, or a custom definition) **or** `definition_yaml` (an inline dynamic flow definition — the full YAML, same schema as catalog definitions); provide exactly one of the two. Catalog starts use the guarded v2 protocol. A `definition_yaml` flow has extra constraints: every participant must declare `workspace: none` and a concrete, priced model (no `default`), the server clamps `limits`/`mcp` to its own caps rather than trusting the definition, may reject the whole thing with a coded error, and requires a server with dynamic flows enabled. Also provide `target_kind`, `target_ref`, `target_title`, and `context`. Requester context (session ID, cwd, repo root, owner, name) is resolved automatically from the environment. Returns a `flow_run_id`. A single-participant definition starts its first round eagerly; a multi-participant definition starts **round-less** — the response carries no round, and each declared role's agent launches lazily on its first `send_to_participant`. Optional `mode`: by default, when the daemon runs on the same machine and the selected vendor declares a borrowed-context containment strategy, the participant sees current tracked, dirty, and non-ignored untracked checkout content; the safety boundary is vendor-specific. Cursor runs in a daemon-owned snapshot refreshed before follow-up rounds, while direct borrowing is reserved for runtimes with a native read-only tool clamp. The capability is advertised for whatever build of the vendor CLI is installed — it is not gated on a per-version validation record, since a vendor auto-update would then silently drop the participant back onto a stale committed base. It IS gated per platform where the containment boundary has not been measured: **GitHub Copilot advertises borrowed review only on macOS/ARM64, and only on a daemon that can both enforce an OS sandbox (`sandbox-exec`) and broker a token** (`COPILOT_GITHUB_TOKEN`/`GH_TOKEN`/`GITHUB_TOKEN`) — widening its tool surface enough to read a snapshot also widens what a read tool can be pointed at, so the boundary is an OS sandbox rather than the vendor's own permission prompts, and that sandbox grants neither your keychain nor your Copilot state (see [Borrowed-context Copilot reviews](#borrowed-context-copilot-reviews)). Where any of the three is missing, a Copilot borrowed request returns `vendor_containment_unreadable` naming the daemon and the remedies; `mode="context-only"` works and is the remedy. Pass `mode="context-only"` to opt out. Claude review flows currently use owned worktrees because it declares no borrowed-review containment strategy. Optional `vendor`: for reserved `spec-review`/`code-review`, explicitly selects the reviewer independently of the driver; when omitted the server uses `Flows:Review:DefaultVendor`. Custom single-participant catalog definitions retain their authored vendor unless explicitly overridden. The server records requested/applied vendor plus selection source, and rejects an unavailable or uncertified vendor without silent fallback. Dynamic (`definition_yaml`) flows reject a top-level vendor override because each participant declares its own vendor. Optional `model`: a per-run reviewer **model** override for a single-participant catalog review — REQUIRES `vendor` (the model is interpreted against that vendor; there is no vendor→model table anywhere in the CLI, so `model` without `vendor` is rejected locally before any request is sent) and is rejected on a `definition_yaml` (dynamic) or multi-participant start (each participant already pins its own model). Pass the vendor's own model id or alias verbatim, case-sensitive — the CLI never translates, canonicalizes, or guesses it.
+- **`start_flow`** — start a new flow from either `definition_id` (the server's flow-definition catalog, e.g. `spec-review`, `code-review`, or a custom definition) **or** `definition_yaml` (an inline dynamic flow definition — the full YAML, same schema as catalog definitions); provide exactly one of the two. Catalog starts use the guarded v2 protocol. A `definition_yaml` flow has extra constraints: every participant must declare `workspace: none` and a concrete, priced model (no `default`), the server clamps `limits`/`mcp` to its own caps rather than trusting the definition, may reject the whole thing with a coded error, and requires a server with dynamic flows enabled. Also provide `target_kind`, `target_ref`, `target_title`, and `context`. Requester context (session ID, cwd, repo root, owner, name) is resolved automatically from the environment. Returns a `flow_run_id`. A single-participant definition starts its first round eagerly; a multi-participant definition starts **round-less** — the response carries no round, and each declared role's agent launches lazily on its first `send_to_participant`. Optional `mode`: by default, when the daemon runs on the same machine and the selected vendor declares a borrowed-context containment strategy, the participant sees current tracked, dirty, and non-ignored untracked checkout content; the safety boundary is vendor-specific. Cursor runs in a daemon-owned snapshot refreshed before follow-up rounds, while direct borrowing is reserved for runtimes with a native read-only tool clamp. The capability is advertised for whatever build of the vendor CLI is installed — it is not gated on a per-version validation record, since a vendor auto-update would then silently drop the participant back onto a stale committed base. It IS gated per platform where the containment boundary has not been measured: **GitHub Copilot advertises borrowed review only on macOS/ARM64, and only on a daemon that can both enforce an OS sandbox (`sandbox-exec`) and broker a token** (`COPILOT_GITHUB_TOKEN`/`GH_TOKEN`/`GITHUB_TOKEN`) — widening its tool surface enough to read a snapshot also widens what a read tool can be pointed at, so the boundary is an OS sandbox rather than the vendor's own permission prompts, and that sandbox grants neither your keychain nor your Copilot state (see [Borrowed-context Copilot reviews](#borrowed-context-copilot-reviews)). Where any of the three is missing, a Copilot borrowed request returns `vendor_containment_unreadable` naming the daemon and the remedies; `mode="context-only"` works and is the remedy. Pass `mode="context-only"` to opt out. Claude review flows currently use owned worktrees because it declares no borrowed-review containment strategy. Optional `vendor`: for reserved `spec-review`/`code-review`, explicitly selects the reviewer independently of the driver; when omitted, resolution falls through to the definition's authored vendor when it declares one, then to your saved `flows.reviewer_vendor` preference (applied via one automatic retry, with the response saying so), and finally a coded `reviewer_vendor_required` if neither is set — ask the user which reviewer vendor to use, pass it explicitly, and offer to save it with `kcap config set flows.reviewer_vendor <vendor>`. Custom single-participant catalog definitions retain their authored vendor unless explicitly overridden. The server records requested/applied vendor plus selection source, and rejects an unavailable or uncertified vendor without silent fallback. Dynamic (`definition_yaml`) flows reject a top-level vendor override because each participant declares its own vendor. Optional `model`: a per-run reviewer **model** override for a single-participant catalog review — REQUIRES `vendor` (the model is interpreted against that vendor; there is no vendor→model table anywhere in the CLI, so `model` without `vendor` is rejected locally before any request is sent) and is rejected on a `definition_yaml` (dynamic) or multi-participant start (each participant already pins its own model). Pass the vendor's own model id or alias verbatim, case-sensitive — the CLI never translates, canonicalizes, or guesses it.
 - **`send_to_participant`** — send a follow-up message to a participant role declared by the flow definition (single-participant definitions use `"reviewer"`; the server rejects an unknown role, naming the valid ones). One round runs at a time per role — a second send to a busy role is rejected naming the busy round, while other roles remain addressable. Returns the new round's findings.
 - **`get_flow_status`** — get the current status (running, waiting, completed, failed) and last result of a flow run.
 - **`close_flow`** — mark a completed flow run as closed.
@@ -842,7 +842,7 @@ The daemon advertises posture support to the server at connect. A server that of
 >
 > **Restart your coding agent after upgrading, too — not just for hooks.** A harness reads each MCP server's tool schema **once**, when it connects, and caches it for the life of that session. Upgrading kcap replaces the binary on disk but cannot reach into a harness process that has already connected, so a session started before the upgrade keeps offering the **old** tool schema — missing any parameter the new version added. kcap deliberately does not try to hot-reload a cached schema; restarting is the supported recovery.
 >
-> This matters most for review flows. If `start_review_flow` is missing its `vendor` parameter, the agent driving it cannot name a reviewer, and the server falls back to its configured `Flows:Review:DefaultVendor` — so asking for "a review by Claude" can quietly get you a different reviewer. **The server cannot detect this**: an omitted `vendor` looks exactly like a caller who wanted the default, so the request carries no trace of the name you asked for. (The `client_upgrade_required`, `flow_client_protocol_required` and `flow_client_protocol_unsupported` rejections guard a different problem — *protocol* skew — and do not fire here.) The only guard in this window is the kcap flow skills, which instruct agents never to claim a named reviewer ran when they could not send `vendor`, and to tell you to restart instead.
+> This matters most for review flows. If `start_review_flow` is missing its `vendor` parameter, the agent driving it cannot name a reviewer, and resolution falls through to the flow definition's authored vendor (if it declares one) or the agent's saved `flows.reviewer_vendor` preference (if it doesn't) — so asking for "a review by Claude" can quietly get you a different reviewer. **The server cannot detect this**: an omitted `vendor` looks exactly like a caller who wanted whatever it resolves to, so the request carries no trace of the name you asked for. (The `client_upgrade_required`, `flow_client_protocol_required` and `flow_client_protocol_unsupported` rejections guard a different problem — *protocol* skew — and do not fire here.) The only guard in this window is the kcap flow skills, which instruct agents never to claim a named reviewer ran when they could not send `vendor`, and to tell you to restart instead.
 >
 > **Recovery:** upgrade kcap, then end the harness session and start a fresh one — or, in harnesses that expose it, reconnect the `kcap-flows` MCP server. To confirm the new schema is live, check that your client lists the server as connected (`claude mcp list`, `codex mcp get kcap-flows`, `gemini mcp list`, `cursor-agent mcp list`, `opencode mcp list`) and that `start_review_flow` now advertises `vendor`.
 

kcap/skills/agent-flows/SKILL.md

@@ -44,20 +44,26 @@ Once the user has explicitly opted into a flow (see above), pick the `definition
 
 For the reserved `spec-review` and `code-review` aliases, reviewer-vendor language is role-bound:
 pass the one vendor explicitly named as the reviewer, ignore driver-harness mentions, honor
-negation, omit the vendor when none is named, and ask when multiple candidates remain. Custom
+negation, omit the vendor when none is named, and ask when multiple candidates remain. Omitting
+`vendor` resolves to the definition's authored vendor when it declares one, then to your saved
+`flows.reviewer_vendor` preference, then (with nothing saved) a `reviewer_vendor_required`
+response — see `review-flows`' "Choosing the reviewer vendor" for the full chain. Custom
 catalog definitions keep their authored vendors unless an explicit single-participant override is
 requested; dynamic definitions always carry vendors per participant and reject a top-level override.
 
 ### If `start_flow` has no `vendor` parameter
 
 Your harness is holding an MCP tool schema it cached before kcap was upgraded, and kcap cannot
 refresh a schema the harness has already cached. A parameter you cannot see is one you cannot send:
-**do not start the flow and then report that the named reviewer ran** — without `vendor` the server
-applies its own configured default, which may be a different vendor. Nothing server-side can catch
-this for you: an omitted `vendor` is indistinguishable from a caller who deliberately wanted the
-default, so the request carries no trace of the name the user asked for. Tell the user to restart
-the harness session (or reconnect the `kcap-flows` MCP server) and start a fresh task; start without
-`vendor` only if they then explicitly ask for the server default.
+**do not start the flow and then report that the named reviewer ran** — without `vendor` you get
+whatever the flow definition's authored vendor resolves to, or — for a vendor-less definition — an
+automatic retry against your saved `flows.reviewer_vendor` preference, or a
+`reviewer_vendor_required` response if none is saved; none of these is guaranteed to be the vendor
+the user named. Nothing server-side can catch this for you: an omitted `vendor` is indistinguishable
+from a caller who deliberately wants that resolved vendor, so the request carries no trace of the
+name the user asked for. Tell the user to restart the harness session (or reconnect the
+`kcap-flows` MCP server) and start a fresh task; start without `vendor` only if they then
+explicitly ask you to proceed with whatever vendor the definition or saved preference resolves to.
 
 Canonical reviewer aliases for reserved review flows: Claude / Claude Code → `claude`; Codex /
 OpenAI Codex → `codex`; Cursor / cursor-agent → `cursor`; GitHub Copilot / Copilot CLI → `copilot`;
@@ -204,7 +210,7 @@ report completion to user
 
 | Tool | Required args | Optional args | When to call |
 |---|---|---|---|
-| `start_flow` | Exactly one of `definition_id` (catalog id, e.g. `spec-review`, `code-review`, or a custom catalog id) or `definition_yaml` (inline dynamic definition — see "Composing a dynamic flow"); plus `target_kind` (what is being worked on: `spec`, `code`, `pr`, `branch`, `file`, etc.), `target_ref` (a path, branch name, or PR URL/number that identifies the target), `target_title` (short human-readable title), `context` (background context: what to focus on, constraints, definition of done) | `vendor` (reserved aliases only — explicit reviewer vendor; omit for server default), `model` (reserved aliases only — explicit reviewer model override; REQUIRES `vendor`, rejected on dynamic/multi-participant starts), `instructions`, `mode` (`context-only` — optional; by default the participant's worktree is mirrored from THIS SESSION's project directory, not from the directory you are working in. Pass `context-only` to opt out and treat the submitted context as authoritative) | Once, at the start of a flow task. |
+| `start_flow` | Exactly one of `definition_id` (catalog id, e.g. `spec-review`, `code-review`, or a custom catalog id) or `definition_yaml` (inline dynamic definition — see "Composing a dynamic flow"); plus `target_kind` (what is being worked on: `spec`, `code`, `pr`, `branch`, `file`, etc.), `target_ref` (a path, branch name, or PR URL/number that identifies the target), `target_title` (short human-readable title), `context` (background context: what to focus on, constraints, definition of done) | `vendor` (reserved aliases only — explicit reviewer vendor; omit to use the definition's authored vendor, or your saved `flows.reviewer_vendor` preference if it declares none), `model` (reserved aliases only — explicit reviewer model override; REQUIRES `vendor`, rejected on dynamic/multi-participant starts), `instructions`, `mode` (`context-only` — optional; by default the participant's worktree is mirrored from THIS SESSION's project directory, not from the directory you are working in. Pass `context-only` to opt out and treat the submitted context as authoritative) | Once, at the start of a flow task. |
 | `send_to_participant` | `flow_run_id`, `participant` (role name declared in the flow definition's `participants` map; single-participant definitions use `reviewer` — an unknown role is rejected, naming the valid ones), `message` | `instructions`, `async` (defaults to `true`) | After addressing a non-clean result for that role, or to launch a role for the first time. Pass the same `flow_run_id`, the role's name, and the updated message. |
 | `get_flow_status` | `flow_run_id` | — | Poll or check the current status of a flow run (running, waiting, completed, failed). |
 | `close_flow` | `flow_run_id` | — | Only after the definition's clean signal — or when abandoning the task early; the run otherwise stays open until closed. |