From 05fd1c6ff1eddfb1bc01c65df0c195b53f2a5c5d Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 12:42:22 +0200 Subject: [PATCH 01/16] Spec catalogue-carried flow guidance Moves when-to-offer and how-to-drive knowledge from CLI skills into flow definitions, so operator-published flows are offered like the review built-ins. Co-Authored-By: Claude Opus 5.5 --- ...26-10-03-catalogue-flow-guidance-design.md | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md diff --git a/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md b/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md new file mode 100644 index 0000000000..dadd4a4cf4 --- /dev/null +++ b/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md @@ -0,0 +1,147 @@ +# Catalogue-carried flow guidance + +## Goal + +A flow published to the server catalogue is offered and driven by coding agents exactly the way +`spec-review` and `code-review` are today. The knowledge of *when* to offer a flow and *how* to +drive it moves out of CLI skills and instructions and into the flow definition itself. + +This is the first of three sub-projects toward a CLI that knows nothing about any specific flow: + +| | Sub-project | Depends on | +|---|---|---| +| **A** | Flows describe themselves in the catalogue (this spec) | — | +| B | Each definition sets its participants' contract: outcome vocabulary, sign-off outcome, system prompt | — | +| C | Retire the review surface: `*_review_*` alias tools, `review-flows` skill, built-in id strings | A, B | + +## Current state + +- `GET /api/flows/definitions` returns `id`, `version`, `description`, `is_single_participant` and + participants (`role`, `vendor`, `model`). `description` is the only text a driving agent sees. +- The engine implies the driving loop (submit, fix, resubmit until `clean`, then close), but nothing + authors it. The CLI skills `review-flows`, `agent-flows` and `suggest-review-flow` carry it, along + with the mapping from spec → `spec-review` and code → `code-review`, and the per-flow advice + (commit range, "do not ask the reviewer to run tests"). +- Proactive offering comes from `KcapAgentInstructions.cs` and the `suggest-review-flow` skill, both + review-only. A flow an operator publishes is never offered, and the skills tell agents not to touch + the flows tools unless the user names a flow. + +## Design + +### Server: definition schema + +A new optional top-level `guidance` block: + +```yaml +guidance: + offer: proactive # proactive | on_request; default on_request + when_to_use: >- # plain text, at most 300 characters + ... + driver_guide: | # markdown, at most 8 KB + ... +``` + +- `guidance` is catalogue metadata. It is not projected into `FlowDefinition` and never reaches the + launch path; the run engine is unchanged. +- Publish rejects `offer: proactive` without a `when_to_use`, a field over its cap, and unknown + keys inside `guidance` (the parser's existing strict snake_case rule). +- `flow-definition.schema.json` (admin Monaco editor) gains `guidance` and the missing + `initial_prompt_context_only` / `follow_up_prompt_context_only` keys, so it validates both built-ins. + +### Server: API + +- `RunnableFlowDefinition` gains `offer` and `when_to_use` (additive JSON fields). +- New `GET /api/flows/definitions/{id}`: the listing entry plus `driver_guide`. Unknown, disabled + and deleted ids return 404, matching what `start_flow` refuses. + +### Server: built-ins + +`spec-review.yaml` and `code-review.yaml` gain `guidance` blocks with `offer: proactive`. Their +`when_to_use` and `driver_guide` carry what the CLI skills say about each today: when to offer, what +context to submit (spec path / branch name and explicit commit range, reviewer works offline), how to +iterate, when to close, and for `code-review` that tests are out of scope. `SyncBuiltIns` re-seeds +them at a new version unless an admin has published over them. + +An admin override of a built-in keeps its own YAML. Without a `guidance` block it is `on_request` and +stops being offered; the release notes say so. + +### CLI: session-start flows lane + +- A third lane in `SessionStartCompositeContextProvider`, beside memory and guidelines, plus the + equivalent emitter on Claude's SessionStart path. +- It calls `GET /api/flows/definitions`, keeps `offer: proactive` entries, and renders: + + ``` + Flows you may offer (ask first; never start unprompted): + - code-review: + - spec-review: + Before starting one, call get_flow_definition() and follow its guide. + ``` + +- At most 10 flows with their `when_to_use`, about 2 KB in total; further entries are listed by id only. +- Fails open like the other lanes: an error, timeout, or a server without the fields injects nothing. +- Skipped when the `kcap-flows` MCP server is not registered for the harness: offering a flow the + agent cannot start is worse than not offering it. + +### CLI: flows MCP server + +- `list_flow_definitions` renders `offer` and `when_to_use` per entry. +- New tool `get_flow_definition(definition_id)`, annotated read-only: description, participants and + `driver_guide`. A 404 from an old server that lacks the endpoint, told apart from an unknown id by + the error body, gets a "this server does not publish flow guides" notice. + +### CLI: skills and instructions + +- `agent-flows` becomes the one skill for driving any flow. + - Triggers: the user names a flow, accepts an offered one, or the agent is about to offer one listed + in its session context. + - Workflow: `get_flow_definition` → prepare context per the guide → `start_flow` → rounds via + `send_to_participant` → `close_flow`. + - The `spec-review` / `code-review` specifics are removed; they now come from the guides. + - The description stays within the 1024-character skill cap. +- `suggest-review-flow` is deleted. +- In `KcapAgentInstructions.cs`, the review-offer paragraph becomes one generic line: offer flows + listed in the session context when their *when to use* applies, and ask before starting. +- `review-flows` stays until sub-project C because the alias tools it documents still exist. Its + description points to `agent-flows`. +- `help-mcp.txt` and `README.md` document `get_flow_definition` and the lane. + +## Compatibility + +| CLI | Server | Behaviour | +|---|---|---| +| old | new | New fields ignored; unchanged. | +| new | old | The lane injects nothing, so there are no proactive offers. `get_flow_definition` returns the "not published" notice. Running a named flow is unaffected. | +| new | new | Catalogue-driven offers and guides. | + +The new-CLI/old-server regression is accepted: the server ships first. + +## Delivery + +1. Server PR: schema, parser, admin JSON schema, listing fields, `/definitions/{id}`, built-in guidance. +2. CLI PR: lane and Claude emitter, MCP tool changes, skills, instructions, help text, README. The + listing keeps being parsed as `JsonNode`, so there is no DTO coupling and no submodule bump. + +## Testing + +**Server** +- Parser: a valid `guidance` block, the length caps, `when_to_use` required when proactive, unknown keys rejected. +- Seed: both built-ins carry guidance and re-version; an admin override is left alone. +- Endpoints: listing fields; `/{id}` returns 404 for disabled and deleted definitions. +- `flow-definition.schema.json` validates both built-in YAML files. + +**CLI** +- Lane: + - renders proactive flows only + - applies the flow-count and size caps + - fails open on 5xx, timeout and missing fields + - is skipped without `kcap-flows` +- `get_flow_definition`: rendering, unknown id, and an old server (WireMock). +- Schema conformance and annotation pins for the new tool. +- Skill conformance: `agent-flows` description cap; `suggest-review-flow` absent. + +## Out of scope + +- The outcome vocabulary, sign-off outcome and participant system prompt (sub-project B). +- Removing the alias tools, the `review-flows` skill and the old-server string naming the built-ins (sub-project C). +- Input parameters for flow definitions. From ce620d7aa0f6eebc49c89132342919cc76251db4 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 13:02:26 +0200 Subject: [PATCH 02/16] Serve built-in flow guidance to admin overrides without it Co-Authored-By: Claude Opus 5.5 --- .../2026-10-03-catalogue-flow-guidance-design.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md b/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md index dadd4a4cf4..4620cb6180 100644 --- a/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md +++ b/docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md @@ -62,8 +62,11 @@ context to submit (spec path / branch name and explicit commit range, reviewer w iterate, when to close, and for `code-review` that tests are out of scope. `SyncBuiltIns` re-seeds them at a new version unless an admin has published over them. -An admin override of a built-in keeps its own YAML. Without a `guidance` block it is `on_request` and -stops being offered; the release notes say so. +`SyncBuiltIns` never rewrites an admin-published override of a built-in id, so the guidance reaches an +override at read time instead. For a built-in id whose current definition has no `guidance` block, +the listing and `/definitions/{id}` serve the guidance from the embedded built-in YAML. An override +that authors any `guidance` block is served as written; `guidance: { offer: on_request }` is how an +admin stops agents offering it. Admin-authored YAML is never modified and no version is appended. ### CLI: session-start flows lane @@ -127,6 +130,11 @@ The new-CLI/old-server regression is accepted: the server ships first. **Server** - Parser: a valid `guidance` block, the length caps, `when_to_use` required when proactive, unknown keys rejected. - Seed: both built-ins carry guidance and re-version; an admin override is left alone. +- Guidance fallback for a built-in id: + - an override with no `guidance` serves the embedded guidance + - an override with its own `guidance` serves it as written + - an override with `offer: on_request` is not proactive + - a custom (non-built-in) id with no `guidance` gets no fallback - Endpoints: listing fields; `/{id}` returns 404 for disabled and deleted definitions. - `flow-definition.schema.json` validates both built-in YAML files. From 63eb17ce3a18d163050128c4a15c232588bdd8bf Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 13:29:25 +0200 Subject: [PATCH 03/16] Plan catalogue flow guidance for server and CLI The server plan is drafted here and executes on a kcap-server branch; the CLI plan works against old and new servers. Co-Authored-By: Claude Opus 5.5 --- .../2026-10-03-catalogue-flow-guidance-cli.md | 1344 +++++++++++++++++ ...26-10-03-catalogue-flow-guidance-server.md | 969 ++++++++++++ 2 files changed, 2313 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-cli.md create mode 100644 docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md diff --git a/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-cli.md b/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-cli.md new file mode 100644 index 0000000000..526f4fde77 --- /dev/null +++ b/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-cli.md @@ -0,0 +1,1344 @@ +# Catalogue Flow Guidance — CLI Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Agents learn which catalogue flows to offer, and how to drive them, from the server. The CLI's review-specific offering logic goes away. + +**Architecture:** +- `list_flow_definitions` renders the server's new `offer` and `when_to_use` fields. +- A new read-only `get_flow_definition` tool returns a flow's driver guide. +- A third session-start lane, beside memory and guidelines, injects the proactive flows into every harness's session context, Claude included. +- `agent-flows` becomes the one skill for driving any flow. `suggest-review-flow` and the review-offer paragraph in the agent instructions are removed. + +**Tech Stack:** .NET 10 NativeAOT, `System.Text.Json.Nodes` (no DTOs, so nothing for AOT to trim), TUnit, WireMock.Net. + +**Spec:** `docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md`. The server plan (`docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md`) ships first; this plan works against both old and new servers. + +## Global Constraints + +- Server JSON fields: `offer` (`proactive` | `on_request`), `when_to_use` (≤ 300 characters), `driver_guide` (markdown). +- By-id route: `GET /api/flows/definitions/{id}`. A new server answers an unknown id with a 404 problem body carrying `detail`; an old server answers a routing 404 with no such body. +- Lane caps: at most 10 flows rendered with their `when_to_use`, and a fragment of at most 2048 characters. Any further flows are listed by id only. +- The lane fails open: an error, timeout or old server injects nothing and never fails the hook. +- The lane is skipped when `kcap-flows` is not registered for the harness (`McpServerNudgeAvailability.IsRegisteredFor`). +- Server text is untrusted. Render it through `NextWorkUntrustedText.Render` before it enters session context. +- `review-flows`, the `*_review_*` aliases, `ServerCannotListDefinitions` and the closing hint in `FormatFlowDefinitions` stay; removing them is a later sub-project. +- README and `help-*.txt` change in the same PR as the CLI surface (CLAUDE.md rule). +- Comments are scarce: no ticket ids, no change narration. Commit subject: one imperative clause, ≤ 80 characters, no issue reference unless the user supplies one. +- After the change, `dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release` shows no IL2026/IL3050. + +## Review Focus + +1. **Old server, every surface.** `/api/flows/definitions` without `offer` fields, and `/definitions/{id}` returning a routing 404, must produce no lane block and a clear "does not publish guides" notice, never an error. Pinned in Tasks 1, 2 and 3. +2. **A hostile or sloppy `when_to_use`** (newlines, ``-like tags, 5 KB of text) must not break out of the injected block or blow the 2 KB cap. Pinned in Task 3. +3. **Every other lane disabled but `kcap-flows` registered.** The flows block must still be injected. Three short-circuits currently skip everything when memory and guidelines are both off. Pinned in Tasks 3 and 4. +4. **`kcap-flows` registered, memory lane on, flows endpoint 5xx.** The memory fragment must still be emitted byte-for-byte as before. Pinned in Task 3 (composite) and Task 4 (Claude). +5. **An upgraded user keeps `~/.agents/skills/kcap-suggest-review-flow`** unless the installer prunes it. A stale copy would keep offering review twice. Pinned in Task 5. + +--- + +### Task 1: Render offer and when_to_use in `list_flow_definitions` + +**Files:** +- Modify: `src/Capacitor.Cli/Commands/McpFlowsServer.cs:1552-1591` (`FormatFlowDefinitions`) and the `list_flow_definitions` description (~2573) +- Test: `test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionsTests.cs` + +**Interfaces:** +- Produces: an unchanged signature, `McpFlowsServer.FormatFlowDefinitions(string body) : string?`. + - It renders ` when to use (offer proactively): ` for proactive entries. + - It renders ` when to use (on request): ` when an on-request entry has text. + - It renders nothing extra when the fields are absent. + +- [ ] **Step 1: Write the failing tests.** Append to `McpFlowsServerFlowDefinitionsTests`. + +```csharp + [Test] + public async Task A_proactive_definition_says_when_to_offer_it() { + var text = McpFlowsServer.FormatFlowDefinitions(""" + {"definitions":[{"id":"code-review","version":4,"is_single_participant":true, + "participants":[{"role":"reviewer","model":"default"}], + "offer":"proactive","when_to_use":"After a change\nis complete."}]} + """)!; + + await Assert.That(text).Contains(" when to use (offer proactively): After a change is complete."); + } + + [Test] + public async Task An_on_request_definition_shows_its_when_to_use_without_inviting_an_offer() { + var text = McpFlowsServer.FormatFlowDefinitions(""" + {"definitions":[{"id":"triage","participants":[{"role":"reviewer","model":"default"}], + "offer":"on_request","when_to_use":"When asked to triage."}]} + """)!; + + await Assert.That(text).Contains(" when to use (on request): When asked to triage."); + await Assert.That(text).DoesNotContain("offer proactively"); + } + + [Test] + public async Task A_server_without_guidance_fields_renders_as_before() { + var text = McpFlowsServer.FormatFlowDefinitions(TwoDefinitions)!; + + await Assert.That(text).DoesNotContain("when to use"); + } + + [Test] + public async Task The_listing_points_to_get_flow_definition() { + var tool = McpFlowsServer.BuildToolsList().Single(t => t.Name == "list_flow_definitions"); + + await Assert.That(tool.Description).Contains("get_flow_definition"); + } +``` + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj -- --treenode-filter "/*/*/McpFlowsServerFlowDefinitionsTests/*"` +Expected: the four new tests FAIL; the existing ones pass. + +- [ ] **Step 3: Implement.** In `FormatFlowDefinitions`, directly after the `description` line, insert: + +```csharp + if (Str(definition, "when_to_use") is { Length: > 0 } whenToUse) + sb.AppendLine(Str(definition, "offer") == "proactive" + ? $" when to use (offer proactively): {whenToUse.ReplaceLineEndings(" ").Trim()}" + : $" when to use (on request): {whenToUse.ReplaceLineEndings(" ").Trim()}"); +``` + +In the `list_flow_definitions` tool description, after the sentence ending "…authored vendor (unset means the request or your saved preference decides) and model. ", insert: + +``` +"A definition may also say when to use it, and whether you may offer it unprompted (offer proactively) or only run it when asked (on request). Before starting one, call get_flow_definition for its guide. " + +``` + +- [ ] **Step 4: Run the tests and confirm they pass.** Same command as Step 2. Expected: all pass. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Cli/Commands/McpFlowsServer.cs test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionsTests.cs +git commit -m "Show when to use each flow in list_flow_definitions" +``` + +--- + +### Task 2: `get_flow_definition` tool + +**Files:** +- Modify: `src/Capacitor.Cli/Commands/McpFlowsServer.cs` (new dispatch branch after the `list_flow_definitions` branch at ~449-471; `FormatFlowDefinitionDetail` and `ServerPublishesNoGuides` beside `FormatFlowDefinitions`; a tool entry after `list_flow_definitions` in `BuildToolsList`) +- Create: `test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionDetailTests.cs` +- Modify: `test/Capacitor.Cli.Tests.Unit/Commands/McpToolAnnotationsTests.cs:47-53` +- Modify: `test/Capacitor.Cli.Tests.Integration/McpFlowsServerTests.cs:308` (tool count 12 → 13, plus a name check) + +**Interfaces:** +- Consumes: `GetBoundedAsync(HttpClient, string url, FlowRetryClock)`, `AuthRejectionNotice.ForPersistentUnauthorizedAsync`, `FormatFlowStartError(int, string, bool)` and `McpToolAnnotations.Read`, all existing. +- Produces: + - MCP tool `get_flow_definition` with one required string argument, `definition_id`. + - `internal static string? FormatFlowDefinitionDetail(string body)` + - `internal const string ServerPublishesNoGuides` + +- [ ] **Step 1: Write the failing tests.** Create `McpFlowsServerFlowDefinitionDetailTests.cs`: + +```csharp +using System.Text.Json.Nodes; +using Capacitor.Cli.Commands; +using Capacitor.Cli.Core; +using Capacitor.Cli.PrDetection; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using WireMock.Server; + +namespace Capacitor.Cli.Tests.Unit.Commands; + +public class McpFlowsServerFlowDefinitionDetailTests { + [TempConfigRoot] public required TempConfigRoot Config { get; init; } + + McpFlowsServer Server() => + new(Config.Root, Resolutions.None(Config.Root), AuthFixtures.NewTokenStore(Config.Root), + new FixedCapacitorHttpClient(), NoTelemetry.Startup, router: new GitProviderRouter(), workdir: new WorkingDirectory(AppContext.BaseDirectory), time: TimeProvider.System); + + static JsonObject ToolCall(JsonObject arguments) => new() { + ["params"] = new JsonObject { ["name"] = "get_flow_definition", ["arguments"] = arguments } + }; + + static (string Text, bool IsError) Result(string response) { + var result = JsonNode.Parse(response)!["result"]!; + + return (result["content"]![0]!["text"]!.GetValue(), result["isError"]?.GetValue() ?? false); + } + + static WireMockServer Serving(string id, int status, string body, string contentType = "application/json") { + var server = WireMockServer.Start(); + server.Given(Request.Create().WithPath($"/api/flows/definitions/{id}").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(status).WithHeader("Content-Type", contentType).WithBody(body)); + + return server; + } + + async Task<(string Text, bool IsError)> CallAsync(WireMockServer server, string id) { + using var client = new HttpClient(); + + return Result(await Server().HandleToolCallAsync( + JsonNode.Parse("1")!, ToolCall(new JsonObject { ["definition_id"] = id }), client, server.Url!, cwd: "/r", repoRoot: "/r", repoInfo: null)); + } + + const string CodeReview = """ + {"id":"code-review","version":4,"description":"Review code changes.","is_single_participant":true, + "participants":[{"role":"reviewer","vendor":null,"model":"default"}], + "offer":"proactive","when_to_use":"After a change.","driver_guide":"## What to submit\nThe commit range."} + """; + + [Test] + public async Task Renders_the_guide_under_the_definition() { + using var server = Serving("code-review", 200, CodeReview); + + var (text, isError) = await CallAsync(server, "code-review"); + + await Assert.That(isError).IsFalse(); + await Assert.That(text).Contains("code-review (v4) — single participant"); + await Assert.That(text).Contains(" reviewer: vendor unset"); + await Assert.That(text).Contains("when to use (offer proactively): After a change."); + await Assert.That(text).Contains("Driver guide:\n## What to submit\nThe commit range."); + } + + [Test] + public async Task A_definition_without_a_guide_says_so() { + using var server = Serving("plain", 200, """{"id":"plain","participants":[{"role":"reviewer","model":"default"}],"offer":"on_request"}"""); + + var (text, isError) = await CallAsync(server, "plain"); + + await Assert.That(isError).IsFalse(); + await Assert.That(text).Contains("This definition publishes no driver guide"); + } + + [Test] + public async Task An_unknown_id_is_an_error_naming_the_listing() { + using var server = Serving("nope", 404, """{"status":404,"detail":"Flow definition 'nope' is not available."}""", "application/problem+json"); + + var (text, isError) = await CallAsync(server, "nope"); + + await Assert.That(isError).IsTrue(); + await Assert.That(text).Contains("Flow definition 'nope' is not available."); + await Assert.That(text).Contains("list_flow_definitions"); + } + + [Test] + public async Task An_older_server_without_the_route_is_reported_not_errored() { + using var server = Serving("code-review", 404, "", "text/plain"); + + var (text, isError) = await CallAsync(server, "code-review"); + + await Assert.That(isError).IsFalse(); + await Assert.That(text).IsEqualTo(McpFlowsServer.ServerPublishesNoGuides); + } + + [Test] + public async Task A_catalog_that_has_not_projected_is_a_retryable_refusal() { + using var server = Serving("code-review", 409, """{"error":"server_catching_up","message":"Flows are temporarily unavailable."}"""); + + var (text, isError) = await CallAsync(server, "code-review"); + + await Assert.That(isError).IsTrue(); + await Assert.That(text).Contains(McpFlowsServer.ServerCatchingUpGuidance); + } + + [Test] + public async Task A_missing_definition_id_is_rejected_without_a_request() { + using var server = Serving("x", 200, CodeReview); + using var client = new HttpClient(); + + var (text, isError) = Result(await Server().HandleToolCallAsync( + JsonNode.Parse("1")!, ToolCall(new JsonObject()), client, server.Url!, cwd: "/r", repoRoot: "/r", repoInfo: null)); + + await Assert.That(isError).IsTrue(); + await Assert.That(text).Contains("definition_id is required"); + await Assert.That(server.LogEntries.Count()).IsEqualTo(0); + } + + [Test] + public async Task An_id_is_escaped_into_one_path_segment() { + using var server = Serving("a%2Fb", 404, ""); + using var client = new HttpClient(); + + await Server().HandleToolCallAsync( + JsonNode.Parse("1")!, ToolCall(new JsonObject { ["definition_id"] = "a/b" }), client, server.Url!, cwd: "/r", repoRoot: "/r", repoInfo: null); + + await Assert.That(server.LogEntries.Single().RequestMessage.AbsolutePath).IsEqualTo("/api/flows/definitions/a%2Fb"); + } + + [Test] + public async Task The_tool_is_read_only_and_requires_definition_id() { + var tool = McpFlowsServer.BuildToolsList().Single(t => t.Name == "get_flow_definition"); + + await Assert.That(tool.Annotations).IsEqualTo(McpToolAnnotations.Read); + await Assert.That(tool.InputSchema.Required).IsEquivalentTo(new[] { "definition_id" }); + await Assert.That(tool.Description).Contains("before start_flow"); + } +} +``` + +If `WireMockServer` un-escapes `%2F` when it matches paths, drop the `Serving` in `An_id_is_escaped_into_one_path_segment`, start a bare `WireMockServer.Start()`, and keep the `AbsolutePath` assertion on the log entry. If the log normalises the escape away, assert on `RequestMessage.Url` containing `a%2Fb` instead. + +In `McpToolAnnotationsTests.Reads_are_read_only_and_writes_say_what_they_do`, after the `list_flow_definitions` line, add: + +```csharp + await Assert.That(Tool("kcap-flows", "get_flow_definition").Annotations.ReadOnlyHint).IsTrue(); +``` + +In `McpFlowsServerTests.cs:308`, change `IsEqualTo(12)` to `IsEqualTo(13)` and add: + +```csharp + await Assert.That(names.Contains("get_flow_definition")).IsTrue(); +``` + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj -- --treenode-filter "/*/*/McpFlowsServerFlowDefinitionDetailTests/*"` +Expected: compile errors, because `ServerPublishesNoGuides` is missing. + +- [ ] **Step 3: Implement.** + +Add the dispatch branch directly after the `list_flow_definitions` branch: + +```csharp + if (toolName is "get_flow_definition") { + var definitionId = arguments?["definition_id"] is JsonValue dv && dv.TryGetValue(out string? s) ? s?.Trim() : null; + if (string.IsNullOrEmpty(definitionId)) + return BuildToolResult(id, "Error: definition_id is required — pass an id from list_flow_definitions.", isError: true); + + var lookup = await GetBoundedAsync(client, $"{apiRoot}/api/flows/definitions/{Uri.EscapeDataString(definitionId)}", clock); + if (lookup.Response is null) + return BuildToolResult(id, $"Error: reading flow definition '{definitionId}' {lookup.How}; nothing was started — retry the call.", isError: true); + + using var detailResp = lookup.Response; + var detailBody = await detailResp.Content.ReadAsStringAsync(); + + if (detailResp.StatusCode == HttpStatusCode.Unauthorized) + return BuildToolResult(id, await AuthRejectionNotice.ForPersistentUnauthorizedAsync(store, profiles.Name, apiRoot, time), isError: true); + + // A server that serves the route answers an unknown id with a problem body; a routing 404 has none. + if (detailResp.StatusCode == HttpStatusCode.NotFound) + return ProblemDetail(detailBody) is { } detail + ? BuildToolResult(id, $"Error: {detail} Call list_flow_definitions for the ids this server can start.", isError: true) + : BuildToolResult(id, ServerPublishesNoGuides); + + if (!detailResp.IsSuccessStatusCode) + return BuildToolResult(id, FormatFlowStartError((int)detailResp.StatusCode, detailBody, wasDynamicStart: false), isError: true); + + return FormatFlowDefinitionDetail(detailBody) is { } rendered + ? BuildToolResult(id, rendered) + : BuildToolResult(id, $"Error: unreadable flow definition from GET /api/flows/definitions/{definitionId}.", isError: true); + } +``` + +Use whatever name the surrounding branches use for the tool arguments object. Read the `start_flow` branch, which reads `definition_id`, and reuse its accessor rather than the inline `arguments?[…]` above if one exists. + +Add beside `FormatFlowDefinitions`: + +```csharp + internal const string ServerPublishesNoGuides = + "This server does not publish flow guides (GET /api/flows/definitions/{id} is not available). " + + "Use the definition's description from list_flow_definitions and the agent-flows skill's rules."; + + /// Renders GET /api/flows/definitions/{id}: the listing entry, then the authored driver guide verbatim. + /// Null when the body is not a definition. + internal static string? FormatFlowDefinitionDetail(string body) { + JsonNode? root; + try { root = JsonNode.Parse(body); } catch (JsonException) { return null; } + + if (root is not JsonObject definition || definition["id"] is not JsonValue) return null; + + var listing = FormatFlowDefinitions(new JsonObject { ["definitions"] = new JsonArray(definition.DeepClone()) }.ToJsonString()); + if (listing is null) return null; + + // Keep the entry lines only: drop the "flow_definitions (1):" header and the closing hint. + var lines = listing.Split('\n'); + var entry = string.Join('\n', lines[1..^1]).TrimEnd(); + + var guide = definition["driver_guide"] is JsonValue g && g.TryGetValue(out string? text) && !string.IsNullOrWhiteSpace(text) + ? $"Driver guide:\n{text.Trim()}" + : "This definition publishes no driver guide: follow its description and the agent-flows skill's rules."; + + return $"{entry}\n\n{guide}"; + } + + static string? ProblemDetail(string body) { + try { + return JsonNode.Parse(body) is JsonObject o && o["detail"] is JsonValue v && v.TryGetValue(out string? d) && !string.IsNullOrWhiteSpace(d) ? d : null; + } catch (JsonException) { + return null; + } + } +``` + +`new JsonArray(definition.DeepClone())` uses the `params JsonNode?[]` constructor, which is AOT-safe. A collection expression would not be (see CLAUDE.md). + +Add the tool entry after `list_flow_definitions` in `BuildToolsList`: + +```csharp + new( + "get_flow_definition", + "Read one flow definition's guide before start_flow: what context to submit, how to iterate on its results and when to close. " + + "Read-only and side-effect-free: this does NOT start anything. " + + "Returns the definition's listing entry (participants, when to use it) followed by its authored driver guide. " + + "An unknown, disabled or deleted id is an error — call list_flow_definitions for the ids this server can start. " + + "A server that does not publish guides says so; then rely on the definition's description.", + new( + "object", + new() { + ["definition_id"] = new("string", "Definition id from list_flow_definitions, e.g. 'code-review'.") + }, + ["definition_id"] + ), + McpToolAnnotations.Read + ), +``` + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the Step 2 command, then the `McpToolAnnotationsTests` and `FlowsDriverSchemaConformanceTests` filters. Then run the integration project: +`dotnet run --project test/Capacitor.Cli.Tests.Integration/Capacitor.Cli.Tests.Integration.csproj -- --treenode-filter "/*/*/McpFlowsServerTests/*"` +Expected: all pass. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Cli/Commands/McpFlowsServer.cs test/Capacitor.Cli.Tests.Unit/Commands/ test/Capacitor.Cli.Tests.Integration/McpFlowsServerTests.cs +git commit -m "Add get_flow_definition to read a flow's driver guide" +``` + +--- + +### Task 3: Session-start flows lane + +**Files:** +- Create: `src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs` +- Modify: `src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs:47-58` (`FlowsDisabled`, `AllLanesDisabled`) +- Modify: `src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs` (third lane; generalised `Combine`/`Compose`) +- Modify: `src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs:29` +- Modify: `src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs:30-42` (construct the lane; add `FlowsLaneDisabled`) +- Create: `test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs` +- Modify: `test/Capacitor.Cli.Tests.Unit/SessionStartMemory/GuidelinesLaneAndCompositeTests.cs:106-119` (composite factory gains the lane) + +**Interfaces:** +- Consumes: + - `SessionStartContextFetch.FetchAsync(HttpClient, string url, TimeProvider, CancellationToken) : SessionStartFetchOutcome(HttpStatusCode Status, byte[]? Body, TimeSpan? RetryAfter)` + - `NextWorkUntrustedText.Render(string?, int)` (`Capacitor.Cli.Core.WorkItems`) + - `McpServerNudgeAvailability.IsRegisteredFor(HarnessId, HarnessRegistry, string, string?)` + - `KcapMcpServers.FlowsServerName` +- Produces: + - `SessionStartMemoryContextRequest.FlowsDisabled` (bool, default `true`) + - `SessionStartMemoryContextRequest.AllLanesDisabled` (bool) + - `SessionStartFlowsLane(Func> client, TimeProvider time)`, with: + - `FetchAsync(SessionStartMemoryContextRequest, CancellationToken) : Task` + - `static BuildFragment(JsonNode?) : string?` + - consts `Header`, `Footer`, `MaxDescribed = 10`, `MaxFragmentChars = 2048` + - `SessionStartCompositeContextProvider(resolver, memory, guidelines, flows, time, diagnostic?)` + - `SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId, HarnessRegistry, string? codexConfigPath = null) : bool` + +- [ ] **Step 1: Write the failing tests.** Create `FlowsLaneTests.cs`. Reuse the private `Handler` and `FixedScope` stubs from `GuidelinesLaneAndCompositeTests` (lines 233-251) by moving them to a shared `internal` file in the same folder named `SessionStartStubs.cs`; that move is part of this task. + +```csharp +using System.Net; +using System.Text.Json.Nodes; +using Capacitor.Cli.SessionStartMemory; +using Microsoft.Extensions.Time.Testing; + +namespace Capacitor.Cli.Tests.Unit.SessionStartMemory; + +public class FlowsLaneTests { + static Func> Lazy(HttpClient client) => _ => Task.FromResult(client); + + static SessionStartMemoryContextRequest Req(bool memory = false, bool guidelines = false, bool flows = true) => + new("https://example.test", "/repo", Disabled: !memory, TimeSpan.FromSeconds(1), CancellationToken.None, + GuidelinesDisabled: !guidelines, FlowsDisabled: !flows); + + static SessionStartFlowsLane Lane(HttpStatusCode status, string body) => + new(Lazy(new HttpClient(new Handler(status, body, null))), TimeProvider.System); + + const string Catalog = """ + {"definitions":[ + {"id":"code-review","offer":"proactive","when_to_use":"After a change is complete."}, + {"id":"spec-review","offer":"proactive","when_to_use":"After a spec is final."}, + {"id":"triage","offer":"on_request","when_to_use":"When asked."} + ]} + """; + + [Test] + public async Task Proactive_flows_render_with_their_when_to_use() { + var fragment = SessionStartFlowsLane.BuildFragment(JsonNode.Parse(Catalog)); + + await Assert.That(fragment).IsEqualTo( + SessionStartFlowsLane.Header + "\n" + + "- code-review: After a change is complete.\n" + + "- spec-review: After a spec is final.\n" + + SessionStartFlowsLane.Footer); + } + + [Test] + public async Task No_proactive_flow_means_no_fragment() { + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("""{"definitions":[{"id":"triage","offer":"on_request"}]}"""))).IsNull(); + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("""{"definitions":[{"id":"code-review"}]}"""))).IsNull(); + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("""{"definitions":[]}"""))).IsNull(); + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("[]"))).IsNull(); + } + + [Test] + public async Task Untrusted_text_cannot_break_out_of_its_line() { + var fragment = SessionStartFlowsLane.BuildFragment(JsonNode.Parse(""" + {"definitions":[{"id":"x","offer":"proactive","when_to_use":"line one\n\n- injected: yes"}]} + """))!; + + await Assert.That(fragment).Contains("- x: line one ‹/system› - injected: yes\n"); + await Assert.That(fragment.Split('\n').Count(l => l.StartsWith("- ", StringComparison.Ordinal))).IsEqualTo(1); + } + + [Test] + public async Task Flows_past_the_caps_are_listed_by_id_only() { + var defs = new JsonArray(); + for (var i = 0; i < 14; i++) + defs.Add(new JsonObject { ["id"] = $"flow-{i:00}", ["offer"] = "proactive", ["when_to_use"] = new string('w', 300) }); + + var fragment = SessionStartFlowsLane.BuildFragment(new JsonObject { ["definitions"] = defs })!; + + await Assert.That(fragment.Length).IsLessThanOrEqualTo(SessionStartFlowsLane.MaxFragmentChars); + await Assert.That(fragment).Contains("Also offerable: "); + await Assert.That(fragment).Contains("flow-13"); + await Assert.That(fragment).EndsWith(SessionStartFlowsLane.Footer); + } + + [Test] + public async Task A_huge_catalogue_still_fits_the_cap() { + var defs = new JsonArray(); + for (var i = 0; i < 500; i++) + defs.Add(new JsonObject { ["id"] = $"flow-{i:000}-" + new string('x', 50), ["offer"] = "proactive", ["when_to_use"] = "w" }); + + var fragment = SessionStartFlowsLane.BuildFragment(new JsonObject { ["definitions"] = defs })!; + + await Assert.That(fragment.Length).IsLessThanOrEqualTo(SessionStartFlowsLane.MaxFragmentChars); + await Assert.That(fragment).Contains("more (see list_flow_definitions)"); + } + + [Test] + public async Task An_older_server_404_is_empty_not_retried() { + var result = await Lane(HttpStatusCode.NotFound, "").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.CompleteWithoutContext); + } + + [Test] + public async Task A_server_error_is_retryable() { + var result = await Lane(HttpStatusCode.ServiceUnavailable, "").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.RetryableFailure); + } + + [Test] + public async Task A_listing_without_guidance_fields_is_empty() { + var result = await Lane(HttpStatusCode.OK, """{"definitions":[{"id":"code-review","version":3}]}""").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.CompleteWithoutContext); + } + + [Test] + public async Task The_lane_reads_the_definitions_listing() { + var handler = new Handler(HttpStatusCode.OK, Catalog, null); + var lane = new SessionStartFlowsLane(Lazy(new HttpClient(handler)), TimeProvider.System); + + var result = await lane.FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(handler.Uri!.AbsolutePath).IsEqualTo("/api/flows/definitions"); + } + + [Test] + public async Task With_every_other_lane_off_the_composite_still_injects_flows() { + var composite = Composite(memory: (HttpStatusCode.OK, "[]"), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)); + + var result = await composite.GetAsync(Req(memory: false, guidelines: false, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(result.Fragment!).StartsWith(MemoryIndexEmitter.FragmentMarker + "\n" + SessionStartFlowsLane.Header); + } + + [Test] + public async Task A_failing_flows_lane_leaves_the_memory_fragment_untouched() { + const string memoryBody = """[{"memory_id":"m1","slug":"s","audience":"org","description":"d","kind":"preference"}]"""; + var withFlows = await Composite(memory: (HttpStatusCode.OK, memoryBody), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.InternalServerError, "")) + .GetAsync(Req(memory: true, flows: true)); + var withoutFlows = await Composite(memory: (HttpStatusCode.OK, memoryBody), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, flows: false)); + + await Assert.That(withFlows.Fragment).IsEqualTo(withoutFlows.Fragment); + } + + [Test] + public async Task Memory_comes_first_then_flows() { + const string memoryBody = """[{"memory_id":"m1","slug":"s","audience":"org","description":"d","kind":"preference"}]"""; + + var result = await Composite(memory: (HttpStatusCode.OK, memoryBody), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, flows: true)); + + var fragment = result.Fragment!; + await Assert.That(fragment.IndexOf(MemoryIndexEmitter.FragmentMarker, StringComparison.Ordinal)).IsEqualTo(0); + await Assert.That(fragment.IndexOf(SessionStartFlowsLane.Header, StringComparison.Ordinal)).IsGreaterThan(0); + } + + static SessionStartCompositeContextProvider Composite( + (HttpStatusCode, string) memory, (HttpStatusCode, string) guidelines, (HttpStatusCode, string) flows) { + var scope = new FixedScope(new SessionStartMemoryScope("repo", "machine")); + var time = new FakeTimeProvider(); + + return new SessionStartCompositeContextProvider(scope, + new SessionStartMemoryContextProvider(scope, Lazy(new HttpClient(new Handler(memory.Item1, memory.Item2, null))), time), + new SessionStartGuidelinesLane(Lazy(new HttpClient(new Handler(guidelines.Item1, guidelines.Item2, null))), time), + new SessionStartFlowsLane(Lazy(new HttpClient(new Handler(flows.Item1, flows.Item2, null))), time), + time); + } +} +``` + +The `Req` positional and named arguments must match the record's parameter names (`BaseUrl, Cwd, Disabled, Budget, CancellationToken, GuidelinesDisabled, FlowsDisabled`). Adjust the `Disabled:` label if the compiler disagrees. The `Composite` helper mirrors the factory at `GuidelinesLaneAndCompositeTests.cs:106-119`; copy any extra arguments that factory passes. + +In `GuidelinesLaneAndCompositeTests`, change its composite factory to pass a fourth lane, `new SessionStartFlowsLane(Lazy(new HttpClient(new Handler(HttpStatusCode.NotFound, "", null))), time)`. Its requests leave `FlowsDisabled` at the default `true`, so the lane never runs and those tests keep their expectations. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj -- --treenode-filter "/*/*/FlowsLaneTests/*"` +Expected: compile errors, because `SessionStartFlowsLane` and `FlowsDisabled` are missing. + +- [ ] **Step 3: Implement.** + +In `SessionStartMemoryContracts.cs`, extend the request and replace its long parameter comment with a one-liner: + +```csharp +/// Each lane has its own opt-out. The guidelines and flows flags default to off so a memory-only +/// construction stays memory-only. +internal sealed record SessionStartMemoryContextRequest( + string BaseUrl, + string? Cwd, + bool Disabled, + TimeSpan Budget, + CancellationToken CancellationToken, + bool GuidelinesDisabled = true, + bool FlowsDisabled = true) { + public bool AllLanesDisabled => Disabled && GuidelinesDisabled && FlowsDisabled; +} +``` + +`SessionStartFlowsLane.cs`: + +```csharp +using System.Net; +using System.Text; +using System.Text.Json.Nodes; +using Capacitor.Cli.Core.WorkItems; + +namespace Capacitor.Cli.SessionStartMemory; + +/// The flows lane of the composite SessionStart context: the catalogue flows an operator marked +/// offer: proactive, each with when to offer it. Marker-less, like the guidelines lane. +internal sealed class SessionStartFlowsLane(Func> client, TimeProvider time) { + public const int MaxDescribed = 10; + public const int MaxFragmentChars = 2048; + const int WhenToUseCap = 300; + const int IdCap = 64; + + public const string Header = "Flows you may offer (ask first; never start unprompted):"; + public const string Footer = "Before starting one, call get_flow_definition() and follow its guide."; + + public async Task FetchAsync(SessionStartMemoryContextRequest request, CancellationToken ct) { + var outcome = await SessionStartContextFetch.FetchAsync( + await client(ct), request.BaseUrl.TrimEnd('/') + "/api/flows/definitions", time, ct); + + // A server older than the listing has nothing to offer, now or on a retry. + if (outcome.Status == HttpStatusCode.NotFound) return SessionStartMemoryContextResult.Empty; + if (outcome.Body is null) + return new SessionStartMemoryContextResult(SessionStartMemoryDisposition.RetryableFailure, RetryAfter: outcome.RetryAfter); + + return BuildFragment(JsonNode.Parse(outcome.Body)) is { } fragment + ? new SessionStartMemoryContextResult(SessionStartMemoryDisposition.Ready, fragment) + : SessionStartMemoryContextResult.Empty; + } + + public static string? BuildFragment(JsonNode? root) { + if (root is not JsonObject { } obj || obj["definitions"] is not JsonArray definitions) return null; + + var offered = definitions.OfType() + .Where(d => Str(d, "offer") == "proactive") + .Select(d => (Id: NextWorkUntrustedText.Render(Str(d, "id"), IdCap), When: NextWorkUntrustedText.Render(Str(d, "when_to_use"), WhenToUseCap))) + .Where(f => f.Id.Length > 0) + .ToList(); + if (offered.Count == 0) return null; + + // Described in order until the count or the room runs out; everything after falls back to its id, so the + // server's id order survives. The overflow line has its own fixed reserve, so the block never passes the cap. + var room = MaxFragmentChars - Header.Length - Footer.Length - 2 - OverflowReserve; + var described = new StringBuilder(); + var idsOnly = new List(); + var count = 0; + + foreach (var (id, when) in offered) { + var line = when.Length > 0 ? $"- {id}: {when}\n" : $"- {id}\n"; + if (idsOnly.Count == 0 && count < MaxDescribed && described.Length + line.Length <= room) { + described.Append(line); + count++; + } else { + idsOnly.Add(id); + } + } + + var sb = new StringBuilder(); + sb.Append(Header).Append('\n').Append(described); + if (idsOnly.Count > 0) sb.Append(Overflow(idsOnly)).Append('\n'); + sb.Append(Footer); + + return sb.ToString(); + + static string Overflow(List ids) { + var line = new StringBuilder("Also offerable: "); + var shown = 0; + foreach (var id in ids) { + var piece = shown == 0 ? id : ", " + id; + if (line.Length + piece.Length > OverflowReserve - OverflowTail.Length) break; + line.Append(piece); + shown++; + } + + if (shown < ids.Count) line.Append($" and {ids.Count - shown} more").Append(OverflowTail); + + return line.ToString(); + } + + static string? Str(JsonObject o, string key) => o[key] is JsonValue v && v.TryGetValue(out string? s) ? s : null; + } + + const int OverflowReserve = 400; + const string OverflowTail = " (see list_flow_definitions)"; +} +``` + +`OverflowReserve` is held back from the described lines even when nothing overflows. That keeps the arithmetic one rule, and at typical `when_to_use` lengths ten flows still fit in the remaining ~1500 characters. + +In `SessionStartCompositeContextProvider.cs`: +- Add `SessionStartFlowsLane flows` as the fourth constructor parameter, after `guidelines`. +- Rewrite the summary in one or two lines: one fragment from the memory, guidelines and flows lanes, the memory marker first. +- Replace `GetAsync` through `MaxRetryAfter` with: + +```csharp + public async Task GetAsync(SessionStartMemoryContextRequest request) { + if (request.AllLanesDisabled) return SessionStartMemoryContextResult.Empty; + if (request.Budget <= TimeSpan.Zero) return SessionStartMemoryContextResult.Retry; + + using var expiry = new CancellationTokenSource(request.Budget, time); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(request.CancellationToken, expiry.Token); + + SessionStartMemoryScope scope; + try { + scope = await scopeResolver.ResolveAsync(request.Cwd, request.Budget, cts.Token); + } catch (Exception ex) when (IsFailOpen(ex)) { + diagnostic?.Invoke($"SessionStart scope resolution skipped: {ex.Message}"); + return SessionStartMemoryContextResult.Retry; + } + + // Started before any await so the lanes share the budget in parallel. + var memoryTask = request.Disabled ? null : RunLaneAsync(() => memory.FetchWithScopeAsync(scope, request, cts.Token)); + var guidelinesTask = request.GuidelinesDisabled ? null : RunLaneAsync(() => guidelines.FetchWithScopeAsync(scope, request, cts.Token)); + var flowsTask = request.FlowsDisabled ? null : RunLaneAsync(() => flows.FetchAsync(request, cts.Token)); + + var memoryResult = memoryTask is null ? null : await memoryTask; + SessionStartMemoryContextResult?[] others = [ + guidelinesTask is null ? null : await guidelinesTask, + flowsTask is null ? null : await flowsTask + ]; + + return Combine(memoryResult, others); + } + + async Task RunLaneAsync(Func> lane) { + try { + return await lane(); + } catch (Exception ex) when (IsFailOpen(ex)) { + diagnostic?.Invoke($"SessionStart context lane skipped: {ex.Message}"); + return SessionStartMemoryContextResult.Retry; + } + } + + static SessionStartMemoryContextResult Combine(SessionStartMemoryContextResult? memoryResult, SessionStartMemoryContextResult?[] others) { + var memoryFragment = Ready(memoryResult); + var otherFragments = others.Select(Ready).OfType().ToList(); + + if (memoryFragment is not null || otherFragments.Count > 0) + return new SessionStartMemoryContextResult(SessionStartMemoryDisposition.Ready, Compose(memoryFragment, otherFragments)); + + SessionStartMemoryContextResult?[] all = [memoryResult, ..others]; + var retries = all.Where(r => r?.Disposition == SessionStartMemoryDisposition.RetryableFailure).ToList(); + if (retries.Count == 0) return SessionStartMemoryContextResult.Empty; + + return new SessionStartMemoryContextResult( + SessionStartMemoryDisposition.RetryableFailure, RetryAfter: retries.Max(r => r!.RetryAfter)); + + static string? Ready(SessionStartMemoryContextResult? r) => + r is { Disposition: SessionStartMemoryDisposition.Ready, Fragment: { } f } ? f : null; + } + + /// Marker first: Pi and OpenCode capture stdout only when it opens with it, and only the memory + /// fragment carries it. + static string Compose(string? memoryFragment, IReadOnlyList others) { + var rest = string.Join("\n\n", others); + if (memoryFragment is null) return MemoryIndexEmitter.FragmentMarker + "\n" + rest; + + return others.Count == 0 ? memoryFragment : memoryFragment + "\n\n" + rest; + } +``` + +`retries.Max(r => r!.RetryAfter)` over `TimeSpan?` returns the largest non-null value, or null when every value is null, which is what `MaxRetryAfter` did; delete `MaxRetryAfter`. Keep `IsFailOpen` unchanged. + +In `SessionStartMemoryOrchestrator.cs:29`, replace the line and its three-line comment with: + +```csharp + // Every lane off spends no lease, so a lane switched on mid-session can still inject on a later callback. + if (request.AllLanesDisabled) return null; +``` + +In `SessionStartMemoryHookSupport.cs`: + +```csharp + var memory = new SessionStartMemoryContextProvider(resolver, client, time); + var guidelines = new SessionStartGuidelinesLane(client, time); + var flows = new SessionStartFlowsLane(client, time); + return new SessionStartCompositeContextProvider(resolver, memory, guidelines, flows, time); + } + + /// Offering a flow the agent cannot start is worse than not offering it. + public static bool FlowsLaneDisabled(HarnessId harness, HarnessRegistry harnesses, string? codexConfigPath = null) => + !McpServerNudgeAvailability.IsRegisteredFor(harness, harnesses, KcapMcpServers.FlowsServerName, codexConfigPath); +``` + +Add any `using` directives the compiler asks for (`Capacitor.Cli.Core.Mcp` for `KcapMcpServers`; `HarnessId` and `HarnessRegistry` live in Core). + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the Step 2 command. Then run `--treenode-filter "/*/*/GuidelinesLaneAndCompositeTests/*"`, and the same filter for `SessionStartMemoryOrchestratorTests` if that class exists. Expected: all pass. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Cli/SessionStartMemory/ test/Capacitor.Cli.Tests.Unit/SessionStartMemory/ +git commit -m "Offer proactive catalogue flows in session-start context" +``` + +--- + +### Task 4: Wire the flows lane into every harness hook + +**Files:** +- Modify (non-Claude hooks, each at its `StartMemoryIndexTask` and its caller): + - `src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs` (~142-171, ~377-388) + - `CursorHookCommand.cs` (~583-617) + - `CopilotHookCommand.cs` (~117-124) + - `GeminiHookCommand.cs` (~199-204) + - `KiroHookCommand.cs` (~109-116) + - `OpenCodeHookCommand.cs` (~265-270) + - `AntigravityHookCommand.cs` (~309-314) + - `PiHookCommand.cs` (~331-336) +- Modify: `src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs` (~719-727, ~1177-1200) +- Test: `test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs` (Fixture plus two tests) +- Test: `test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs` (gate helper) + +**Interfaces:** +- Consumes: `SessionStartMemoryHookSupport.CompositeProvider`, `FlowsLaneDisabled`, `SessionStartMemoryContextRequest.FlowsDisabled` / `AllLanesDisabled` (Task 3). + +- [ ] **Step 1: Write the failing tests.** + +In `ClaudeHookCommandTests.Fixture`, add: + +```csharp + public string FlowDefinitionsBody { get; set; } = """{"definitions":[]}"""; + public int FlowDefinitionsRequestCount => _memoryServer.LogEntries.Count(e => e.RequestMessage.Path == "/api/flows/definitions"); +``` + +At the end of `StubMemoryServer()`, add: + +```csharp + _memoryServer.Given(Request.Create().WithPath("/api/flows/definitions").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200) + .WithHeader("Content-Type", "application/json").WithBody(FlowDefinitionsBody)); +``` + +Add the tests beside `session_start_with_only_a_ready_memory_index_emits_just_the_memory_fragment`: + +```csharp + const string ProactiveCatalog = """{"definitions":[{"id":"code-review","offer":"proactive","when_to_use":"After a change is complete."}]}"""; + + [Test, NotInParallel] + public async Task session_start_offers_proactive_flows_when_kcap_flows_is_registered() { + using var absent = new TempDir(); + using var fx = new Fixture(Config.Root) { RespondJson = "{}", FlowDefinitionsBody = ProactiveCatalog }; + fx.RegisterClaudeMcpServer("kcap-flows"); + + var sid = Guid.NewGuid().ToString("N"); + var (exit, stdout) = await RunCapturingStdoutAsync(() => + fx.HandleAsync($$"""{"hook_event_name":"SessionStart","session_id":"{{sid}}","cwd":"{{AbsentCwd(absent)}}","source":"startup"}""")); + + await Assert.That(exit).IsEqualTo(0); + await Assert.That(stdout).Contains("Flows you may offer"); + await Assert.That(stdout).Contains("- code-review: After a change is complete."); + } + + [Test, NotInParallel] + public async Task without_the_flows_mcp_server_flows_are_neither_requested_nor_offered() { + using var absent = new TempDir(); + using var fx = new Fixture(Config.Root) { RespondJson = "{}", FlowDefinitionsBody = ProactiveCatalog }; + + var sid = Guid.NewGuid().ToString("N"); + var (exit, stdout) = await RunCapturingStdoutAsync(() => + fx.HandleAsync($$"""{"hook_event_name":"SessionStart","session_id":"{{sid}}","cwd":"{{AbsentCwd(absent)}}","source":"startup"}""")); + + await Assert.That(exit).IsEqualTo(0); + await Assert.That(fx.FlowDefinitionsRequestCount).IsEqualTo(0); + await Assert.That(stdout).DoesNotContain("Flows you may offer"); + } + + [Test, NotInParallel] + public async Task with_memory_disabled_the_flows_offer_still_arrives() { + using var absent = new TempDir(); + using var fx = new Fixture(Config.Root, profile: new Profile { DisableMemoryIndex = true }) { RespondJson = "{}", FlowDefinitionsBody = ProactiveCatalog }; + fx.RegisterClaudeMcpServer("kcap-flows"); + + var sid = Guid.NewGuid().ToString("N"); + var (_, stdout) = await RunCapturingStdoutAsync(() => + fx.HandleAsync($$"""{"hook_event_name":"SessionStart","session_id":"{{sid}}","cwd":"{{AbsentCwd(absent)}}","source":"startup"}""")); + + await Assert.That(stdout).Contains("Flows you may offer"); + await Assert.That(fx.MemoryIndexRequested).IsFalse(); + } +``` + +Build the `Profile` the way other tests in this file do; search for `DisableMemoryIndex` in the file, and if no test sets it, construct it with the property initializer above. + +Append to `FlowsLaneTests` a gate check that needs no hook: + +```csharp + [Test] + public async Task The_lane_is_off_for_a_harness_without_kcap_flows() { + using var home = new TempDir(); + + await Assert.That(SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Codex, TestHarnesses.Under(home))).IsTrue(); + } +``` + +`TestHarnesses.Under(home)` is the registry the Claude fixture uses. Match its parameter type there; if it takes a `UserHome`, build one from `home.Path` as that fixture does. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj -- --treenode-filter "/*/*/ClaudeHookCommandTests/*"` +Expected: the three new tests FAIL (no flows block; memory-disabled returns early). Everything else passes. + +- [ ] **Step 3: Implement the Claude path.** In `ClaudeHookCommand.cs`, beside `memoryDisabled` (~719): + +```csharp + var flowsDisabled = SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Claude, harnesses); +``` + +Pass it into the task: `StartMemoryIndexTask(nativeSessionId, sessionCwd, memoryDisabled, flowsDisabled, lifecycleReason, budget.Remaining)`. Then rewrite `StartMemoryIndexTask`: + +```csharp + async Task StartMemoryIndexTask( + string? nativeSessionId, + string? cwd, + bool disabled, + bool flowsDisabled, + SessionLifecycleReason reason, + TimeSpan budget) { + if ((disabled && flowsDisabled) || string.IsNullOrEmpty(nativeSessionId) || budget <= TimeSpan.Zero) + return null; + + try { + var store = SessionStartMemoryLeaseStore.Create(config, clock.Time); + var provider = SessionStartMemoryHookSupport.CompositeProvider(router, config, workdir, http.ForMemoryAsync, clock.Time); + + // Claude's guidelines arrive with the SessionStart response, so the composite runs memory and flows only. + return await new SessionStartMemoryOrchestrator(store, provider, clock.Time).GetFragmentAsync( + new SessionMemoryLifecycle(HarnessId.Claude, nativeSessionId, null, + IsTopLevel: true, ClassificationAuthoritative: true, reason, + CallbackMayRepeat: false), + new SessionStartMemoryContextRequest(Url, cwd, disabled, budget, CancellationToken.None, + GuidelinesDisabled: true, FlowsDisabled: flowsDisabled)); + } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { + return null; + } + } +``` + +The envelope assembly needs no change: the composed fragment arrives through `memoryFragment`. + +- [ ] **Step 4: Implement the eight non-Claude hooks.** Make the same three edits in each file. The Codex version is shown in full; the other seven differ only in the harness id and where their flags come from. + +Codex (`CodexHookCommand.cs`): + +(a) `StartMemoryIndexTask` gains `bool flowsDisabled` after `guidelinesDisabled`. Its guard becomes: + +```csharp + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + || budget <= TimeSpan.Zero) + return null; +``` + +(b) The request it builds becomes: + +```csharp + new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); +``` + +(c) The caller (~377-388) passes: + +```csharp + activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Codex, harnesses), + budget.Remaining); +``` + +If the Codex hook already resolves a Codex config path for `IsRegisteredFor` or `WorkItemsNudgeEmitter.ToolsRegisteredFor` elsewhere in the file, pass that same path as `codexConfigPath`. + +Apply (a), (b) and (c) to Cursor, Copilot, Gemini, Kiro, OpenCode, Antigravity and Pi with `HarnessId.Cursor`, `.Copilot`, `.Gemini`, `.Kiro`, `.OpenCode`, `.Antigravity` and `.Pi`. +- Cursor computes `guidelinesDisabled` as a local (~583) and pre-checks it (~584). Add `var flowsDisabled = SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Cursor, harnesses);` beside it and extend that pre-check to all three flags. +- Every hook holds a `harnesses` field. +- The harness-id member names must match the `HarnessId` type; let the compiler confirm them. + +- [ ] **Step 5: Run the tests and confirm they pass.** + +Run the Step 2 command, then the full unit suite: `dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj`. Expected: all pass. A test in the Claude class that registers `kcap-flows` (line ~572) now also triggers a flows GET; the default body is an empty catalogue, so its output does not change. If it asserts `ServerRequestCount`, update the expected count by one and say why in the commit body. + +- [ ] **Step 6: Commit.** + +```bash +git add src/Capacitor.Cli/Commands/Harness/ test/Capacitor.Cli.Tests.Unit/ +git commit -m "Run the flows lane from every harness's SessionStart hook" -m "Claude moves to the composite provider with guidelines off; its guidelines arrive with the SessionStart response." +``` + +--- + +### Task 5: Skills — one generic driver, retire suggest-review-flow + +**Files:** +- Modify: `kcap/skills/agent-flows/SKILL.md` (frontmatter; "When NOT to use"; "Choosing the flow definition"; new "Read the flow's guide first"; rule 8; Workflow; Tool reference) +- Modify: `kcap/skills/review-flows/SKILL.md` (description pointer) +- Delete: `kcap/skills/suggest-review-flow/` +- Modify: `src/Capacitor.Cli.Core/AgentsSkillsInstaller.cs` (`SourceNames`; new `RetiredSourceNames`; prune in `Install` and `Remove`) +- Modify: `src/Capacitor.Cli.Core/Resources/help-plugin.txt:163-165` +- Modify: `test/Capacitor.Cli.Core.Tests.Unit/AgentsSkillsInstallerTests.cs:6` (mirror) and a new prune test +- Delete: `test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs` +- Create: `test/Capacitor.Cli.Tests.Unit/Commands/AgentFlowsSkillConformanceTests.cs` + +**Interfaces:** +- Produces: `AgentsSkillsInstaller.RetiredSourceNames : string[]` = `["suggest-review-flow"]`. + +- [ ] **Step 1: Write the failing tests.** + +Create `AgentFlowsSkillConformanceTests.cs`: + +```csharp +using Capacitor.Cli.Core; + +namespace Capacitor.Cli.Tests.Unit.Commands; + +/// agent-flows is the one skill that drives any catalogue flow, offered ones included. Its folded +/// description is the trigger surface a harness sees; a strict harness drops a skill over 1024 characters. +public class AgentFlowsSkillConformanceTests { + static string SkillText() => File.ReadAllText(Path.Combine(RepoTree.SkillsSource(), "agent-flows", "SKILL.md")); + + static string FoldedDescription() { + var text = SkillText().Replace("\r\n", "\n").Replace("\r", "\n"); + var m = System.Text.RegularExpressions.Regex.Match(text, @"(?m)^description: >-\n((?:^ .*\n)+)"); + return System.Text.RegularExpressions.Regex.Replace(m.Groups[1].Value, @"\s+", " ").Trim(); + } + + [Test] + public async Task Description_stays_within_the_1024_char_skill_limit() { + var len = FoldedDescription().Length; + await Assert.That(len).IsGreaterThan(0); + await Assert.That(len).IsLessThanOrEqualTo(1024); + } + + [Test] + public async Task Description_triggers_on_an_offered_flow_from_session_context() { + var d = FoldedDescription(); + await Assert.That(d).Contains("Flows you may offer"); + await Assert.That(d).Contains("get_flow_definition"); + } + + [Test] + public async Task Body_reads_the_guide_before_starting() => + await Assert.That(SkillText()).Contains("## Read the flow's guide first"); + + [Test] + public async Task Body_names_no_built_in_definition_as_the_default_choice() { + var text = SkillText(); + await Assert.That(text).DoesNotContain("Spec or design document → `definition_id: \"spec-review\"`"); + await Assert.That(text).DoesNotContain("For a code review flow (`definition_id: \"code-review\"`)"); + } + + [Test] + public async Task Suggest_review_flow_is_retired() { + await Assert.That(Directory.Exists(Path.Combine(RepoTree.SkillsSource(), "suggest-review-flow"))).IsFalse(); + await Assert.That(AgentsSkillsInstaller.RetiredSourceNames).Contains("suggest-review-flow"); + } +} +``` + +In `AgentsSkillsInstallerTests.cs`, remove `"suggest-review-flow"` from the mirror array on line 6, and add: + +```csharp + [Test] + public async Task Install_prunes_a_retired_skill_folder() { + using var tmp = new TempDir(); + var target = tmp.CreateDir("skills"); + tmp.CreateFile("skills/kcap-suggest-review-flow/SKILL.md", "stale"); + + await Assert.That(AgentsSkillsInstaller.Install(RepoTree.SkillsSource(), target)).IsTrue(); + await Assert.That(Directory.Exists(Path.Combine(target, "kcap-suggest-review-flow"))).IsFalse(); + } + + [Test] + public async Task Remove_also_removes_a_retired_skill_folder() { + using var tmp = new TempDir(); + var target = tmp.CreateDir("skills"); + tmp.CreateFile("skills/kcap-suggest-review-flow/SKILL.md", "stale"); + + AgentsSkillsInstaller.Remove(target); + + await Assert.That(Directory.Exists(Path.Combine(target, "kcap-suggest-review-flow"))).IsFalse(); + } + + [Test] + public async Task A_retired_name_is_never_a_source_name() => + await Assert.That(AgentsSkillsInstaller.RetiredSourceNames.Intersect(AgentsSkillsInstaller.SourceNames)).IsEmpty(); +``` + +Check `TempDir.CreateFile`'s signature in `test/Capacitor.Tests.Helpers` (relative path plus content, and whether it creates parent directories). If it does not create parents, call `tmp.CreateDir("skills/kcap-suggest-review-flow")` first. + +Delete `test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs`. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: +- `dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj -- --treenode-filter "/*/*/AgentFlowsSkillConformanceTests/*"` +- `dotnet run --project test/Capacitor.Cli.Core.Tests.Unit/Capacitor.Cli.Core.Tests.Unit.csproj -- --treenode-filter "/*/*/AgentsSkillsInstallerTests/*"` + +Expected: compile error (`RetiredSourceNames`), then content failures. + +- [ ] **Step 3: Implement the installer.** In `AgentsSkillsInstaller.cs`: +- Remove `"suggest-review-flow",` from `SourceNames`. +- Add after `SourceNames`: + +```csharp + /// Skills this installer once shipped. Their kcap- folders are deleted on install and remove, + /// so an upgraded user does not keep a skill that no longer matches the tools. + public static readonly string[] RetiredSourceNames = [ + "suggest-review-flow" + ]; +``` + +In `Install`, after the `foreach (var name in SourceNames)` copy loop and before `WriteMarker`: + +```csharp + foreach (var name in RetiredSourceNames) { + var retired = Path.Combine(targetDir, "kcap-" + name); + if (Directory.Exists(retired)) Directory.Delete(retired, recursive: true); + } +``` + +In `Remove`, change `foreach (var name in SourceNames)` to `foreach (var name in SourceNames.Concat(RetiredSourceNames))`. + +In `help-plugin.txt:163-165`, remove `suggest-review-flow,` from the brace list and keep the line wrapping tidy. + +- [ ] **Step 4: Implement the skills.** + +Delete `kcap/skills/suggest-review-flow/` (`git rm -r`). + +In `kcap/skills/agent-flows/SKILL.md`: + +(a) Replace the whole `description: >-` block with: + +```yaml +description: >- + Use this skill to drive a structured agent *flow*: a separate hosted + participant agent runs a definition from the server's flow catalogue and + iterates with you until sign-off. Use it when the user names a flow or + definition id ("run the code-review flow", "start a flow", "use flow + definition X"), when the user accepts a flow you offered, or when you are + about to offer one listed under "Flows you may offer" in your session + context because its when-to-use applies. Before starting a flow, call + get_flow_definition and follow its guide. Do NOT use this skill (and do NOT + call the flows MCP tools) for an ordinary request such as "review my PR", + "do X for me", or "check this over" where the user wants you to do the work + yourself — do it directly instead. To start a hosted agent that works on its + own, use `start-agents`. +``` + +(b) In "## When NOT to use this skill / these tools", replace the final paragraph ("Only start a flow when the user explicitly asks…") with: + +```markdown +Only start a flow when the user explicitly asks for one — e.g. "start a flow", "run the code-review flow", "use flow definition X", or "re-review after I address the findings" via a flow — or accepts a flow you offered. Offer a flow only when your session context lists it under **Flows you may offer** and its when-to-use applies to what just happened; ask, and never start it before the user says yes. Skip the offer when the user asked you to do the work yourself, or while you are mid-task. +``` + +(c) In "## Choosing the flow definition", replace the two bullets that begin "Spec or design document →" and "Code changes or a pull request →" with one bullet: + +```markdown +- The flow the user named, or the one you offered and they accepted. +``` + +Keep the `list_flow_definitions` bullet and everything after it in that section, including the pinned `### If \`start_flow\` has no \`vendor\` parameter` subsection, unchanged. + +(d) Insert a new section directly before "## Composing a dynamic flow": + +```markdown +## Read the flow's guide first + +Call `get_flow_definition(definition_id)` before `start_flow`. The guide is written by whoever published the flow: what to put in `context`, which target to name, how to iterate on its results and when to close. Where it differs from the generic rules below, follow the guide. A server that does not publish guides says so; then work from the definition's description and the rules below. +``` + +(e) Replace Core rule 8 with: + +```markdown +8. **Ask the participant only for what the flow's guide allows.** If the guide says CI covers tests, do not ask the participant to run them; participant feedback is on what the flow exists to judge. +``` + +(f) At the top of "## Workflow", before the first code block, insert: + +```markdown +Every flow starts the same way: + +``` +get_flow_definition(definition_id) + → read the driver guide; prepare the context it asks for +``` +``` + +Render the inner fence as a real nested code block in the file: a plain paragraph followed by a fenced block. + +(g) In the "## Tool reference" table, add a row after `list_flow_definitions`: + +```markdown +| `get_flow_definition` | `definition_id` | — | Before `start_flow`, every time. Read-only: returns the definition's participants and when to use it, then its authored driver guide (what to submit, how to iterate, when to close). An unknown, disabled or deleted id is an error; an older server reports that it publishes no guides. | +``` + +In `kcap/skills/review-flows/SKILL.md`, append one sentence to the end of the `description: >-` block (keep it folded and under 1024 characters): + +``` + For any other catalogue flow, or a flow offered from your session context, + use `agent-flows`. +``` + +- [ ] **Step 5: Run the tests and confirm they pass.** + +Run both Step 2 commands, then the `FlowsDriverSchemaConformanceTests` filter (it pins the `agent-flows` vendor heading block). Expected: all pass. + +- [ ] **Step 6: Commit.** + +```bash +git add -A kcap/skills src/Capacitor.Cli.Core/AgentsSkillsInstaller.cs src/Capacitor.Cli.Core/Resources/help-plugin.txt test/Capacitor.Cli.Core.Tests.Unit/AgentsSkillsInstallerTests.cs test/Capacitor.Cli.Tests.Unit/Commands/ +git commit -m "Drive every catalogue flow through agent-flows" -m "suggest-review-flow is pruned from installed skill trees, so an upgraded user is not offered review twice." +``` + +--- + +### Task 6: Agent instructions, help text, README + +**Files:** +- Modify: `src/Capacitor.Cli.Core/Instructions/KcapAgentInstructions.cs:45-52` (the "When you **finish implementing a change**" paragraph) +- Modify: `test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs:11-12` +- Modify: `src/Capacitor.Cli.Core/Resources/help-mcp.txt:80-87` (generic-flow tools) +- Modify: `README.md`: + - the `### Flows MCP server (for agents)` bullets (~604-610), with a `get_flow_definition` bullet after line 608 + - a lane paragraph after the bullets + - the skills table (~1515-1530): delete the `kcap-suggest-review-flow` row and update the `kcap-agent-flows` row + +- [ ] **Step 1: Write the failing test.** In `AgentInstructionsWriterTests.cs`, replace lines 11-12 with: + +```csharp + await Assert.That(KcapAgentInstructions.Body).Contains("Flows you may offer"); + await Assert.That(KcapAgentInstructions.Body).Contains("get_flow_definition"); + await Assert.That(KcapAgentInstructions.Body).DoesNotContain("proactively OFFER an independent second-harness review"); +``` + +- [ ] **Step 2: Run the test and confirm it fails.** + +Run: `dotnet run --project test/Capacitor.Cli.Core.Tests.Unit/Capacitor.Cli.Core.Tests.Unit.csproj -- --treenode-filter "/*/*/AgentInstructionsWriterTests/*"` +Expected: FAIL. + +- [ ] **Step 3: Implement.** + +In `KcapAgentInstructions.Body`, replace the paragraph starting "When you **finish implementing a change**" through "…isn't finished." with: + +``` + When your session context lists **Flows you may offer**, OFFER one whose *when to use* fits what + just happened — for example on finishing a change or finalizing a spec — even if the user didn't + ask. Start it with `kcap-flows` `start_flow` ONLY after the user accepts; never auto-start it. + Before starting, call `kcap-flows` `get_flow_definition` and follow its guide. Skip the offer when + the user asked you to do the work yourself, or while you're mid-task. +``` + +Keep the preceding "When asked to **request or submit a review**" paragraph unchanged; the review aliases still exist. + +In `help-mcp.txt`, extend the generic-flow tools block: + +``` + Generic-flow tools (catalog + dynamic definitions): + list_flow_definitions List the definitions this server can start, with when + to use each and whether agents may offer it unprompted. + get_flow_definition Read one definition's driver guide: what to submit, + how to iterate, when to close. + start_flow …(unchanged) +``` + +Add after the `Claude Code auto-registers …` paragraph: + +``` + When `kcap-flows` is registered for a harness, the SessionStart hook lists the + catalogue flows an operator marked `offer: proactive`, so the agent can offer + them at the right moment. Nothing is listed against a server that predates + flow guidance. +``` + +In `README.md`, after the `list_flow_definitions` bullet in `### Flows MCP server (for agents)`: + +```markdown +- **`get_flow_definition`** — read one definition's driver guide before starting it: its participants, when to use it, and the authored guidance on what context to submit, how to iterate and when to close. Read-only. An unknown, disabled or deleted id is an error; a server that predates flow guides is reported as publishing none. +``` + +Append to the `list_flow_definitions` bullet: ` Each entry also carries when to use it and whether agents may offer it unprompted (\`offer: proactive\`) or run it only on request.` + +After the tool bullets, add: + +```markdown +**Offered flows.** When `kcap-flows` is registered for a harness, the SessionStart hook adds a short "Flows you may offer" block listing the catalogue flows marked `offer: proactive`, each with when to use it. The agent offers one at that moment and starts it only after you accept. The built-in `spec-review` and `code-review` flows ship as proactive; an admin controls any flow's offer through the `guidance` block of its definition. +``` + +In the skills table, delete the `kcap-suggest-review-flow` row and change the `kcap-agent-flows` description cell to: `Drive any catalogue flow — named by you or offered from session context — following its driver guide`. + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the Step 2 command, then the full Core unit suite, `dotnet run --project test/Capacitor.Cli.Core.Tests.Unit/Capacitor.Cli.Core.Tests.Unit.csproj`. Expected: all pass, including `PluginCommand*Tests`, which embed the instructions body. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Cli.Core/Instructions/KcapAgentInstructions.cs src/Capacitor.Cli.Core/Resources/help-mcp.txt README.md test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs +git commit -m "Point agents at catalogue-offered flows instead of review" +``` + +--- + +### Task 7: Verification + +- [ ] **Step 1: Build the solution.** `dotnet build Capacitor.slnx`. Expected: 0 errors and 0 warnings (IDE0005 unused usings are build errors). If the build hangs past 10 minutes, run `dotnet build-server shutdown` and rebuild. +- [ ] **Step 2: Check AOT.** `dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release 2>&1 | grep -E 'IL[23][01][0-9]{2}'`. Expected: no output. +- [ ] **Step 3: Run the suites in the foreground, one at a time, with `--no-build`.** + - `test/Capacitor.Cli.Core.Tests.Unit` + - `test/Capacitor.Cli.Tests.Unit` + - `test/Capacitor.Cli.Tests.Integration` + + Expected: all pass. +- [ ] **Step 4: Scan for Linear ids.** `bash scripts/check-linear-ids.sh`. Expected: clean. +- [ ] **Step 5: Read the README diff end to end.** Check that `suggest-review-flow` appears nowhere outside `docs/`: `rtk proxy grep -rn "suggest-review-flow" src test kcap README.md npm` should only match `RetiredSourceNames`, its tests, and `AgentFlowsSkillConformanceTests`. diff --git a/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md b/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md new file mode 100644 index 0000000000..07750d1214 --- /dev/null +++ b/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md @@ -0,0 +1,969 @@ +# Catalogue Flow Guidance — Server Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Flow definitions carry authored guidance (`offer`, `when_to_use`, `driver_guide`). The definitions listing exposes it, and a new by-id endpoint returns the driver guide. + +**Architecture:** +- `guidance` is parsed into `FlowDefinitionSpec` and stored inside the existing `spec_json`, so no new column or migration is needed. It never enters the engine's `FlowDefinition`. +- `IFlowDefinitions` gains two reads that pair the engine definition with its guidance. For a built-in id whose current spec authors no guidance, they fall back to the embedded built-in YAML. + +**Tech Stack:** .NET 10, YamlDotNet, EF Core (Postgres), minimal APIs, TUnit + NSubstitute, Testcontainers. + +**Spec:** `docs/superpowers/specs/2026-10-03-catalogue-flow-guidance-design.md` (kcap-cli repo). Copy it onto the server branch alongside this plan. + +**Repository:** `kurrent-io/kcap-server`. Run `git submodule update --init` in the fresh worktree before building, because the server test build depends on `src/cli`. + +## Global Constraints + +- YAML keys are snake_case; the parser rejects unknown keys, including inside `guidance`. +- `guidance.offer` ∈ {`proactive`, `on_request`}. It defaults to `on_request`. +- `guidance.when_to_use`: at most 300 characters. It is required when `offer` is `proactive`. +- `guidance.driver_guide`: markdown, at most 8192 characters. +- Guidance is metadata only. It never reaches `FlowDefinition`, the prompt renderer or a launch. +- Admin-authored YAML is never modified and no version is appended on the admin's behalf. Fallback is read-time only. +- HTTP JSON is snake_case: `offer`, `when_to_use`, `driver_guide`. +- One type per file, named after the type. +- Comments: scarce. No ticket ids, no change narration, no spec coordinates. +- Commit subject: one imperative clause, at most 80 characters. No ticket prefix unless the user supplies one. +- Every TUnit assertion is `await`ed. Filter with `--treenode-filter`. + +## Review Focus + +1. **A stored `spec_json` written before this change** (no `Guidance` property) must still deserialize, list, start and describe. It gets null guidance, or the built-in fallback for a built-in id. Pinned in Task 3. +2. **`guidance: {}` or a bare `guidance:` key** must parse to `on_request` with no text, or to no guidance at all, rather than throwing a null-reference error. Pinned in Task 1. +3. **A disabled or deleted built-in** must not be described through the fallback. The by-id endpoint returns 404, exactly as `start_flow` refuses it. Pinned in Task 3. +4. **Whitespace-only `when_to_use` with `offer: proactive`** must be rejected like a missing value, not stored as blank text that agents then get offered. Pinned in Task 1. +5. **`GET /api/flows/definitions/{id}` on a catalog still catching up** must answer the retryable 409 `server_catching_up`, never 404. A 404 would read as "no such flow" and stop the driver. Pinned in Task 4. + +--- + +### Task 1: Guidance types and parser + +**Files:** +- Create: `src/Capacitor.Server.Core/Flows/FlowGuidance.cs` +- Create: `src/Capacitor.Server.Core/Flows/FlowGuidanceOffer.cs` +- Modify: `src/Capacitor.Server.Services/Flows/FlowDefinitionSpec.cs:12-19` (trailing `Guidance` parameter) +- Modify: `src/Capacitor.Server.Services/Flows/FlowDefinitionYamlParser.cs` (`Validate`, `RawDefinition`, new `RawGuidance`, new `ValidateGuidance`) +- Test: `test/Capacitor.Server.Tests.Flows/FlowDefinitionYamlParserTests.cs` + +**Interfaces:** +- Produces: + - `Capacitor.Flows.FlowGuidance(string Offer, string? WhenToUse, string? DriverGuide)` with `bool IsProactive`. + - `Capacitor.Flows.FlowGuidanceOffer.Proactive` / `.OnRequest` (string consts). + - `FlowDefinitionSpec.Guidance` (`FlowGuidance?`, trailing, default null). + - `FlowDefinitionYamlParser.MaxWhenToUseLength = 300`, `MaxDriverGuideLength = 8192`. + +- [ ] **Step 1: Write the failing tests.** Append to `FlowDefinitionYamlParserTests`. `Valid` is the existing legacy-shape constant. + +```csharp + [Test] + public async Task Guidance_block_is_parsed() { + var yaml = Valid + """ + + guidance: + offer: proactive + when_to_use: After finishing a change. + driver_guide: | + ## Context + Send the commit range. + """; + + var guidance = FlowDefinitionYamlParser.Parse(yaml).Guidance!; + + await Assert.That(guidance.Offer).IsEqualTo(FlowGuidanceOffer.Proactive); + await Assert.That(guidance.IsProactive).IsTrue(); + await Assert.That(guidance.WhenToUse).IsEqualTo("After finishing a change."); + await Assert.That(guidance.DriverGuide).IsEqualTo("## Context\nSend the commit range."); + } + + [Test] + public async Task No_guidance_block_parses_to_null() => + await Assert.That(FlowDefinitionYamlParser.Parse(Valid).Guidance).IsNull(); + + [Test] + public async Task An_empty_guidance_block_is_on_request_with_no_text() { + var guidance = FlowDefinitionYamlParser.Parse(Valid + "\nguidance: {}\n").Guidance!; + + await Assert.That(guidance.Offer).IsEqualTo(FlowGuidanceOffer.OnRequest); + await Assert.That(guidance.WhenToUse).IsNull(); + await Assert.That(guidance.DriverGuide).IsNull(); + } + + [Test] + public async Task A_bare_guidance_key_parses_to_null() => + await Assert.That(FlowDefinitionYamlParser.Parse(Valid + "\nguidance:\n").Guidance).IsNull(); + + [Test] + [Arguments("guidance:\n offer: proactive\n")] + [Arguments("guidance:\n offer: proactive\n when_to_use: \" \"\n")] + public async Task Proactive_without_when_to_use_is_rejected(string block) => + await Assert.That(() => FlowDefinitionYamlParser.Parse(Valid + "\n" + block)) + .Throws() + .WithMessageContaining("guidance.when_to_use is required"); + + [Test] + public async Task Unknown_offer_is_rejected() => + await Assert.That(() => FlowDefinitionYamlParser.Parse(Valid + "\nguidance:\n offer: always\n")) + .Throws() + .WithMessageContaining("guidance.offer 'always'"); + + [Test] + public async Task When_to_use_over_the_cap_is_rejected() { + var yaml = Valid + $"\nguidance:\n when_to_use: {new string('a', FlowDefinitionYamlParser.MaxWhenToUseLength + 1)}\n"; + + await Assert.That(() => FlowDefinitionYamlParser.Parse(yaml)) + .Throws() + .WithMessageContaining("guidance.when_to_use must be at most 300"); + } + + [Test] + public async Task Driver_guide_over_the_cap_is_rejected() { + var yaml = Valid + $"\nguidance:\n driver_guide: {new string('a', FlowDefinitionYamlParser.MaxDriverGuideLength + 1)}\n"; + + await Assert.That(() => FlowDefinitionYamlParser.Parse(yaml)) + .Throws() + .WithMessageContaining("guidance.driver_guide must be at most 8192"); + } + + [Test] + public async Task Unknown_key_inside_guidance_is_rejected() => + await Assert.That(() => FlowDefinitionYamlParser.Parse(Valid + "\nguidance:\n bogus_key: 1\n")) + .Throws(); + + [Test] + public async Task Guidance_round_trips_through_spec_json() { + var spec = FlowDefinitionYamlParser.Parse(Valid + "\nguidance:\n offer: proactive\n when_to_use: Now.\n"); + + var back = FlowDefinitionSpecJson.Deserialize(FlowDefinitionSpecJson.Serialize(spec)); + + await Assert.That(back.Guidance).IsEqualTo(spec.Guidance); + } +``` + +If `Valid` ends without a trailing newline, the `"\n" +` prefixes keep the appended block at column 0. If `Valid` already contains a `completion:` or `mcp:` block, appending a top-level key after it is still valid YAML. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Server.Tests.Flows/Capacitor.Server.Tests.Flows.csproj -- --treenode-filter "/*/*/FlowDefinitionYamlParserTests/*"` +Expected: compile errors, because `Guidance`, `FlowGuidanceOffer` and `MaxWhenToUseLength` do not exist. + +- [ ] **Step 3: Implement.** + +`src/Capacitor.Server.Core/Flows/FlowGuidanceOffer.cs`: + +```csharp +namespace Capacitor.Flows; + +public static class FlowGuidanceOffer { + public const string Proactive = "proactive"; + public const string OnRequest = "on_request"; +} +``` + +`src/Capacitor.Server.Core/Flows/FlowGuidance.cs`: + +```csharp +namespace Capacitor.Flows; + +/// What a driving agent is told about a definition before starting it. Catalog metadata: it never +/// reaches a launch or a participant prompt. +public sealed record FlowGuidance(string Offer, string? WhenToUse, string? DriverGuide) { + public bool IsProactive => Offer == FlowGuidanceOffer.Proactive; +} +``` + +In `FlowDefinitionSpec.cs`, extend the record parameter list. Leave the rest of the file unchanged; old `spec_json` rows deserialize the missing property as null. + +```csharp +public sealed record FlowDefinitionSpec( + string Id, + string? Description, + IReadOnlyList Participants, + FlowLimitsSpec? Limits, + FlowCompletionSpec? Completion, + IReadOnlyList? Mcp = null, + FlowGuidance? Guidance = null +) { +``` + +In `FlowDefinitionYamlParser.cs`: + +Add the constants beside `MinParticipants`/`MaxParticipants`: + +```csharp + public const int MaxWhenToUseLength = 300; + public const int MaxDriverGuideLength = 8192; +``` + +In `Validate`, replace the last two lines (`var mcp = …; return new FlowDefinitionSpec(…);`) with: + +```csharp + var mcp = raw.Mcp is null ? null : NormalizeMcp(raw.Mcp); + var guidance = raw.Guidance is null ? null : ValidateGuidance(raw.Guidance); + + return new FlowDefinitionSpec(id, raw.Description, participants, limits, completion, mcp, guidance); +``` + +Add after `NormalizeMcp`: + +```csharp + static FlowGuidance ValidateGuidance(RawGuidance raw) { + var offer = string.IsNullOrWhiteSpace(raw.Offer) ? FlowGuidanceOffer.OnRequest : raw.Offer.Trim(); + if (offer is not (FlowGuidanceOffer.Proactive or FlowGuidanceOffer.OnRequest)) + throw new FlowDefinitionValidationException($"guidance.offer '{raw.Offer}' must be 'proactive' or 'on_request'."); + + var whenToUse = string.IsNullOrWhiteSpace(raw.WhenToUse) ? null : raw.WhenToUse.Trim(); + var driverGuide = string.IsNullOrWhiteSpace(raw.DriverGuide) ? null : raw.DriverGuide.TrimEnd(); + + // A proactive flow is offered by its when_to_use line alone, so an offer with nothing to say is an authoring error. + if (offer == FlowGuidanceOffer.Proactive && whenToUse is null) + throw new FlowDefinitionValidationException("guidance.when_to_use is required when guidance.offer is 'proactive'."); + if (whenToUse is { Length: > MaxWhenToUseLength }) + throw new FlowDefinitionValidationException($"guidance.when_to_use must be at most {MaxWhenToUseLength} characters."); + if (driverGuide is { Length: > MaxDriverGuideLength }) + throw new FlowDefinitionValidationException($"guidance.driver_guide must be at most {MaxDriverGuideLength} characters."); + + return new FlowGuidance(offer, whenToUse, driverGuide); + } +``` + +Add `public RawGuidance? Guidance { get; set; }` to `RawDefinition`. Add the nested class beside the other raw types: + +```csharp + sealed class RawGuidance { public string? Offer { get; set; } public string? WhenToUse { get; set; } public string? DriverGuide { get; set; } } +``` + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the same command as in Step 2. Expected: every `FlowDefinitionYamlParserTests` test passes, including the existing ones. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Server.Core/Flows/FlowGuidance.cs src/Capacitor.Server.Core/Flows/FlowGuidanceOffer.cs src/Capacitor.Server.Services/Flows/FlowDefinitionSpec.cs src/Capacitor.Server.Services/Flows/FlowDefinitionYamlParser.cs test/Capacitor.Server.Tests.Flows/FlowDefinitionYamlParserTests.cs +git commit -m "Parse an optional guidance block on flow definitions" +``` + +--- + +### Task 2: Built-in guidance + +**Files:** +- Modify: `src/Capacitor.Server.Services/Flows/Catalog/BuiltIns/spec-review.yaml` +- Modify: `src/Capacitor.Server.Services/Flows/Catalog/BuiltIns/code-review.yaml` +- Modify: `src/Capacitor.Server.Services/Flows/BuiltInFlowDefinitions.cs` (summary, `GuidanceFor`) +- Test: `test/Capacitor.Server.Services.Tests/Flows/BuiltInFlowDefinitionsTests.cs` +- Test: `test/Capacitor.Server.Services.Tests/Flows/Catalog/Commands/FlowCatalogBuiltInSyncTests.cs` + +**Interfaces:** +- Consumes: `FlowGuidance` and `FlowDefinitionSpec.Guidance` (Task 1). +- Produces: `BuiltInFlowDefinitions.GuidanceFor(string id) : FlowGuidance?`, which returns null for a non-built-in id. + +- [ ] **Step 1: Write the failing tests.** + +Append to `BuiltInFlowDefinitionsTests`: + +```csharp + [Test] + [Arguments("spec-review")] + [Arguments("code-review")] + public async Task Each_built_in_is_offered_proactively_with_a_driver_guide(string id) { + var guidance = new BuiltInFlowDefinitions().GuidanceFor(id)!; + + await Assert.That(guidance.IsProactive).IsTrue(); + await Assert.That(guidance.WhenToUse).IsNotNull(); + await Assert.That(guidance.DriverGuide!).Contains("commit range"); + await Assert.That(guidance.DriverGuide!).Contains("close_flow"); + } + + [Test] + public async Task Only_code_review_rules_out_running_tests() { + var shipped = new BuiltInFlowDefinitions(); + + await Assert.That(shipped.GuidanceFor("code-review")!.DriverGuide!).Contains("Do not ask the reviewer to run tests"); + await Assert.That(shipped.GuidanceFor("spec-review")!.DriverGuide!).DoesNotContain("run tests"); + } + + [Test] + public async Task A_non_built_in_id_has_no_guidance() => + await Assert.That(new BuiltInFlowDefinitions().GuidanceFor("my-flow")).IsNull(); +``` + +Append to `FlowCatalogBuiltInSyncTests`. `Sync`, `Create` and `Shipped` are existing helpers in that file. + +```csharp + /// A built-in seeded before it carried guidance is republished at the next version, so the stored + /// row gains the guidance without an operator publishing anything. + [Test] + public async Task A_built_in_stored_without_guidance_is_republished_with_it() { + var (svc, store) = Create(); + var withoutGuidance = Shipped.Documents.Select(StripGuidance).ToArray(); + await Sync(svc, withoutGuidance); + store.AppendedEvents.Clear(); + + await Sync(svc); + + var events = store.AppendedEvents.Select(e => e.Payload).Cast().ToArray(); + await Assert.That(events.Select(e => e.Version).Distinct()).IsEquivalentTo(new[] { 2 }); + await Assert.That(events.All(e => FlowDefinitionSpecJson.Deserialize(e.SpecJson).Guidance is { IsProactive: true })).IsTrue(); + } + + static string StripGuidance(string yaml) => yaml[..yaml.IndexOf("\nguidance:", StringComparison.Ordinal)] + "\n"; +``` + +If `FakeEventStore.AppendedEvents` is not a mutable list, record its count before the second sync and `Skip(count)` instead of calling `Clear()`. `StripGuidance` relies on `guidance:` being the last top-level block in each built-in file, which Step 3 sets up. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Server.Services.Tests/Capacitor.Server.Services.Tests.csproj -- --treenode-filter "/*/*/BuiltInFlowDefinitionsTests/*"` +Expected: compile error, because `GuidanceFor` is missing. + +- [ ] **Step 3: Implement.** + +Append to `spec-review.yaml` as the last top-level block: + +```yaml +guidance: + offer: proactive + when_to_use: >- + After you finalize a design spec or implementation plan, before implementation starts. Offer an + independent review by a separate hosted reviewer; never start it without the user's yes. Skip it + when the user asked you to review the spec yourself. + driver_guide: | + ## Before starting + - Call `list_reviewer_vendors` and recommend a reviewer that can run for this repository, preferring a vendor other than your own harness. Pass `vendor` only for a reviewer the user named. + + ## What to submit + - `target_kind: spec`, `target_ref`: the spec's repository-relative path, `target_title`: its title. + - `context`: the spec path, the branch name and an explicit commit range (`..`), plus what you want scrutinised. Never `origin/main...HEAD`. + - The reviewer works offline in a mirror of this session's project directory. Inline facts instead of linking to GitHub issues or other pages. If the spec lives in another worktree or machine, inline it or pass `mode: context-only`. + + ## Iterating + - `findings`: address each one (revise the spec, or explain why not), commit, then `send_to_participant` with `participant: reviewer`, the new commit range and what changed per finding. + - `clean`: call `close_flow`, then report to the user. + - `unclear` reading `participant_died` or `participant_stopped`: resend; the fresh reviewer remembers nothing, so restate the context. `participant_parked`: resend; it resumes where it was. + - Never start a second flow for the same spec. Recover a lost `flow_run_id` with `get_flow_status(wait: true)`. +``` + +Append to `code-review.yaml` as the last top-level block: + +```yaml +guidance: + offer: proactive + when_to_use: >- + After a code change is functionally complete: tests pass, or you are about to commit, open a PR + or merge. Offer an independent review by a separate hosted reviewer; never start it without the + user's yes. Skip it when the user asked you to review the code yourself, or mid-task. + driver_guide: | + ## Before starting + - Call `list_reviewer_vendors` and recommend a reviewer that can run for this repository, preferring a vendor other than your own harness. Pass `vendor` only for a reviewer the user named. + + ## What to submit + - `target_kind: pr` with the PR number as `target_ref`, or `target_kind: branch` with the branch name; `target_title`: a one-line summary. + - `context`: the branch name and an explicit commit range (`..`), plus what changed and what to focus on. Never `origin/main...HEAD`. + - The reviewer works offline in a mirror of this session's project directory. Inline facts instead of linking to GitHub or other pages. If the change lives in another worktree or machine, inline the diff or pass `mode: context-only`. + - Do not ask the reviewer to run tests; CI covers test execution. + + ## Iterating + - `findings`: address each one (fix it, or explain why not), commit, then `send_to_participant` with `participant: reviewer`, the new commit range and what changed per finding. + - `clean`: call `close_flow`, then report to the user. + - `unclear` reading `participant_died` or `participant_stopped`: resend; the fresh reviewer remembers nothing, so restate the context. `participant_parked`: resend; it resumes where it was. + - Never start a second flow for the same change. Recover a lost `flow_run_id` with `get_flow_status(wait: true)`. +``` + +In `BuiltInFlowDefinitions.cs`, replace the class summary and add `GuidanceFor` after `RawYamlFor`: + +```csharp +/// The shipped built-in definitions, loaded from embedded YAML at version 1: the catalog's seed. No flow +/// resolves from here; the one read is guidance, so an operator override of a built-in that authors none is +/// described with the shipped guidance instead of losing it. +``` + +```csharp + public FlowGuidance? GuidanceFor(string id) => + _loaded.TryGetValue(id, out var l) ? l.Spec.Guidance : null; +``` + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run: `dotnet run --project test/Capacitor.Server.Services.Tests/Capacitor.Server.Services.Tests.csproj -- --treenode-filter "/*/*/BuiltInFlowDefinitionsTests/*"` +Run the same command with `FlowCatalogBuiltInSyncTests`. +Expected: all tests pass. The existing sync tests compare against `Shipped`, so they pick up the new YAML. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Server.Services/Flows/Catalog/BuiltIns/ src/Capacitor.Server.Services/Flows/BuiltInFlowDefinitions.cs test/Capacitor.Server.Services.Tests/Flows/BuiltInFlowDefinitionsTests.cs test/Capacitor.Server.Services.Tests/Flows/Catalog/Commands/FlowCatalogBuiltInSyncTests.cs +git commit -m "Ship driver guidance with the built-in review flows" -m "Deploying republishes spec-review and code-review at their next version unless an admin owns them." +``` + +--- + +### Task 3: Described reads with built-in fallback + +**Files:** +- Create: `src/Capacitor.Server.Core/Flows/DescribedFlowDefinition.cs` +- Modify: `src/Capacitor.Server.Core/Flows/IFlowDefinitions.cs` (two methods) +- Modify: `src/Capacitor.Server.Services/Flows/FlowDefinitions.cs` (constructor, the two methods, `StoredDefinition.IsBuiltIn`) +- Modify: `test/Capacitor.Server.TestHelpers/Fakes/ShippedFlowDefinitions.cs` (implement the two methods) +- Modify: any other `IFlowDefinitions` implementer the compiler reports (check `src/Capacitor.Server/Flows/FlowDefinitionResolver.cs`) +- Test: `test/Capacitor.Server.Services.Tests/Flows/FlowDefinitionsTests.cs` + +**Interfaces:** +- Consumes: `FlowGuidance`, `FlowDefinitionSpec.Guidance` (Task 1); `BuiltInFlowDefinitions.GuidanceFor` (Task 2). +- Produces: + - `Capacitor.Flows.DescribedFlowDefinition(FlowDefinition Definition, FlowGuidance? Guidance)` + - `IFlowDefinitions.ListRunnableDescribedAsync(CancellationToken) : Task>`, ordered by id, enabled and undeleted only. + - `IFlowDefinitions.GetRunnableDescribedAsync(string id, CancellationToken) : Task`. It throws `FlowDefinitionNotRunnableException` for an unknown, disabled, deleted or unreadable definition, and `FlowCatalogNotReadyException` when the catalog cannot catch up. + +- [ ] **Step 1: Write the failing tests.** + +In `FlowDefinitionsTests.Setup`, change the construction to `_catalog = new FlowDefinitions(_contexts, _barrier, new BuiltInFlowDefinitions());`. + +Append the tests below. `SpecReviewYaml`, `CodeReviewYaml` and `Yaml(id)` are the file's existing helpers; `FlowCatalogSeed.Definition(rawYaml, isBuiltIn, publishedBy, …)` is the shared seed. + +```csharp + const string OverrideWithoutGuidance = """ + id: code-review + reviewer: + workspace: none + rounds: + initial_prompt: Operator review. + follow_up_prompt: Operator follow-up. + """; + + static string OverrideWith(string guidanceBlock) => OverrideWithoutGuidance + "\n" + guidanceBlock; + + [Test] + public async Task An_override_of_a_built_in_without_guidance_is_described_with_the_shipped_guidance() { + await FlowCatalogSeed.WriteAsync(_db, seed => seed.Definition(OverrideWithoutGuidance, isBuiltIn: true, publishedBy: "admin")); + + var described = await _catalog.GetRunnableDescribedAsync("code-review"); + + await Assert.That(described.Guidance).IsEqualTo(new BuiltInFlowDefinitions().GuidanceFor("code-review")); + await Assert.That(described.Definition.Participants[0].InitialPrompt).IsEqualTo("Operator review."); + } + + [Test] + public async Task An_override_with_its_own_guidance_is_served_as_written() { + await FlowCatalogSeed.WriteAsync(_db, seed => + seed.Definition(OverrideWith("guidance:\n offer: proactive\n when_to_use: Operator words.\n"), isBuiltIn: true, publishedBy: "admin")); + + var described = await _catalog.GetRunnableDescribedAsync("code-review"); + + await Assert.That(described.Guidance!.WhenToUse).IsEqualTo("Operator words."); + await Assert.That(described.Guidance.DriverGuide).IsNull(); + } + + [Test] + public async Task An_override_set_to_on_request_is_not_proactive() { + await FlowCatalogSeed.WriteAsync(_db, seed => + seed.Definition(OverrideWith("guidance:\n offer: on_request\n"), isBuiltIn: true, publishedBy: "admin")); + + var listed = await _catalog.ListRunnableDescribedAsync(); + + await Assert.That(listed.Single().Guidance!.IsProactive).IsFalse(); + } + + [Test] + public async Task A_custom_definition_without_guidance_gets_no_fallback() { + await FlowCatalogSeed.WriteAsync(_db, seed => seed.Definition(Yaml("aaa-flow"))); + + var listed = await _catalog.ListRunnableDescribedAsync(); + + await Assert.That(listed.Single().Guidance).IsNull(); + } + + [Test] + public async Task A_disabled_built_in_is_not_described() { + await FlowCatalogSeed.WriteAsync(_db, seed => { + var codeReview = seed.Definition(CodeReviewYaml, isBuiltIn: true); + codeReview.SetEnabled(false, DateTimeOffset.UnixEpoch, logPosition: 2); + }); + + await Assert.That(() => _catalog.GetRunnableDescribedAsync("code-review")).Throws(); + await Assert.That(await _catalog.ListRunnableDescribedAsync()).IsEmpty(); + } + + [Test] + public async Task A_spec_stored_before_guidance_existed_still_describes() { + var legacySpecJson = """{"Id":"aaa-flow","Description":null,"Participants":[{"Name":"reviewer","Vendor":null,"Model":"default","Workspace":"None","Rounds":{"InitialPrompt":"a","FollowUpPrompt":"b","Result":null}}],"Limits":null,"Completion":null,"Mcp":null}"""; + await FlowCatalogSeed.WriteAsync(_db, seed => seed.DefinitionWithSpec("aaa-flow", legacySpecJson, rawYaml: null, isBuiltIn: false, publishedBy: null)); + + var described = await _catalog.GetRunnableDescribedAsync("aaa-flow"); + + await Assert.That(described.Definition.Id).IsEqualTo("aaa-flow"); + await Assert.That(described.Guidance).IsNull(); + } +``` + +Check `DefinitionWithSpec`'s exact parameter names in `test/Capacitor.Server.TestHelpers/Persistence/FlowCatalogSeed.cs:44-59` and match them. Check how the stored `Workspace` enum is spelled by serializing a parsed spec once; it is `JsonStringEnumConverter` with the default policy, so the member name is used. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Server.Services.Tests/Capacitor.Server.Services.Tests.csproj -- --treenode-filter "/*/*/FlowDefinitionsTests/*"` (needs a running container engine) +Expected: compile errors, because the constructor arity and the two methods are missing. + +- [ ] **Step 3: Implement.** + +`src/Capacitor.Server.Core/Flows/DescribedFlowDefinition.cs`: + +```csharp +namespace Capacitor.Flows; + +public sealed record DescribedFlowDefinition(FlowDefinition Definition, FlowGuidance? Guidance); +``` + +Add to `IFlowDefinitions`: + +```csharp + /// , each paired with its guidance. + Task> ListRunnableDescribedAsync(CancellationToken ct = default); + + /// , paired with its guidance. + Task GetRunnableDescribedAsync(string id, CancellationToken ct = default); +``` + +In `FlowDefinitions.cs`: + +```csharp +sealed class FlowDefinitions( + IDbContextFactory contexts, + IFlowCatalogProjectionBarrier projection, + BuiltInFlowDefinitions builtIns + ) : IFlowDefinitions { +``` + +```csharp + public async Task> ListRunnableDescribedAsync(CancellationToken ct = default) { + await CatchUpAsync(ct); + + await using var context = await contexts.CreateDbContextAsync(ct); + + var rows = await context.Definitions.Runnable() + .OrderBy(d => d.DefinitionId) + .Select(d => new { d.DefinitionId, d.Version, d.SpecJson, d.IsBuiltIn }) + .ToListAsync(ct); + + return [..rows.Select(r => Describe(r.DefinitionId, r.Version, r.SpecJson, r.IsBuiltIn)) + .OfType()]; + } + + public async Task GetRunnableDescribedAsync(string id, CancellationToken ct = default) { + var stored = await FindAsync(id, ct); + + if (stored is null) { + await CatchUpAsync(ct); + stored = await FindAsync(id, ct) + ?? throw new FlowDefinitionNotRunnableException($"Flow definition '{id}' is not available."); + } + + if (!stored.Runnable) + throw new FlowDefinitionNotRunnableException($"Flow definition '{id}' is not available."); + + return Describe(id, stored.Version, stored.SpecJson, stored.IsBuiltIn) + ?? throw new FlowDefinitionNotRunnableException($"Flow definition '{id}' has an unreadable spec."); + } + + // IsBuiltIn survives an operator re-publish, so an override of a built-in that authors no guidance keeps the shipped one. + DescribedFlowDefinition? Describe(string id, int version, string specJson, bool isBuiltIn) { + var spec = FlowDefinitionSpecJson.TryDeserialize(specJson); + if (spec is null) return null; + + return new DescribedFlowDefinition(spec.ToDefinition(version), spec.Guidance ?? (isBuiltIn ? builtIns.GuidanceFor(id) : null)); + } +``` + +Change `FindAsync`'s projection to `new StoredDefinition(d.Version, d.SpecJson, d.Enabled, d.DeletedAt, d.IsBuiltIn)`, and the record to: + +```csharp + record StoredDefinition(int Version, string SpecJson, bool Enabled, DateTimeOffset? DeletedAt, bool IsBuiltIn) { + public bool Runnable => Enabled && DeletedAt is null; + } +``` + +`BuiltInFlowDefinitions` is already a singleton in `FlowCatalogModule.AddFlowCatalog`, so DI resolves the new parameter. + +In `ShippedFlowDefinitions`: + +```csharp + public Task> ListRunnableDescribedAsync(CancellationToken ct = default) => + Task.FromResult>( + [.._shipped.Definitions.Select(d => new DescribedFlowDefinition(d, _shipped.GuidanceFor(d.Id)))]); + + public Task GetRunnableDescribedAsync(string id, CancellationToken ct = default) => + Task.FromResult(new DescribedFlowDefinition(Runnable(id), _shipped.GuidanceFor(id))); +``` + +Build the solution (`dotnet build Kurrent.Capacitor.slnx`) and implement the two methods on every other `IFlowDefinitions` implementer the compiler names. A decorator delegates both methods to its inner instance. + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the Step 2 command. Expected: all `FlowDefinitionsTests` pass, the existing ones included. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Server.Core/Flows/DescribedFlowDefinition.cs src/Capacitor.Server.Core/Flows/IFlowDefinitions.cs src/Capacitor.Server.Services/Flows/FlowDefinitions.cs test/ +git commit -m "Describe runnable flows with their guidance" +``` + +--- + +### Task 4: Listing fields and the by-id endpoint + +**Files:** +- Modify: `src/Capacitor.Api.Public.Abstractions/Flows/RunnableFlowDefinition.cs` (two trailing parameters) +- Create: `src/Capacitor.Api.Public.Abstractions/Flows/FlowDefinitionDetail.cs` +- Modify: `src/Capacitor.Api.Public/Flows/FlowEndpointHandlers.cs` (`ListDefinitions`, `Describe`, new `GetDefinition`) +- Modify: `src/Capacitor.Api.Public/Flows/FlowEndpoints.cs:87-90` (new route) +- Modify: `src/Capacitor.Server/ServerJsonContext.cs:44-62` (register the response types) +- Modify: `test/Capacitor.Server.Tests.Diagnostics/OpenApi/schemas/{minimal,dedicated,workos}/public.expected.yaml` (regenerated) +- Test: `test/Capacitor.Server.Tests.Flows/FlowEndpointsTests.cs` + +**Interfaces:** +- Consumes: `IFlowDefinitions.ListRunnableDescribedAsync` / `GetRunnableDescribedAsync`, `DescribedFlowDefinition` (Task 3). +- Produces, on the wire: + - `GET /api/flows/definitions` → each definition gains `offer` (string, always present, `on_request` when there is no guidance) and `when_to_use` (string or null). + - `GET /api/flows/definitions/{definitionId}` → `{id, version, description, is_single_participant, participants, offer, when_to_use, driver_guide}`, or one of: + - 404 problem with `detail`: unknown, disabled, deleted or unreadable + - 409 `{error: "server_catching_up", …}` + - 401 + +- [ ] **Step 1: Write the failing tests.** Append to `FlowEndpointsTests`. + +```csharp + static FlowDefinition Def(string id) => + new(id, 3, "Review code changes", [ + new FlowParticipantDefinition("reviewer", null, "default", FlowWorkspacePolicy.MirrorRequester, "initial", "follow-up") + ]); + + static readonly FlowGuidance Guided = new(FlowGuidanceOffer.Proactive, "After a change.", "## Guide"); + + [Test] + public async Task ListDefinitions_carries_offer_and_when_to_use_but_never_the_driver_guide() { + var definitions = Substitute.For(); + definitions.ListRunnableDescribedAsync(Arg.Any()) + .Returns(new List { new(Def("code-review"), Guided), new(Def("plain"), null) }); + + var result = await Handlers(Substitute.For(), definitions: definitions) + .ListDefinitions(NewHttpContext(githubId: "42"), CancellationToken.None); + + var listed = (result.Result as Ok)!.Value!.Definitions; + await Assert.That(listed[0].Offer).IsEqualTo("proactive"); + await Assert.That(listed[0].WhenToUse).IsEqualTo("After a change."); + await Assert.That(listed[1].Offer).IsEqualTo("on_request"); + await Assert.That(listed[1].WhenToUse).IsNull(); + } + + [Test] + public async Task GetDefinition_returns_the_entry_with_its_driver_guide() { + var definitions = Substitute.For(); + definitions.GetRunnableDescribedAsync("code-review", Arg.Any()) + .Returns(new DescribedFlowDefinition(Def("code-review"), Guided)); + + var result = await Handlers(Substitute.For(), definitions: definitions) + .GetDefinition("code-review", NewHttpContext(githubId: "42"), CancellationToken.None); + + var detail = (result.Result as Ok)!.Value!; + await Assert.That(detail.Id).IsEqualTo("code-review"); + await Assert.That(detail.Offer).IsEqualTo("proactive"); + await Assert.That(detail.DriverGuide).IsEqualTo("## Guide"); + await Assert.That(detail.Participants.Single().Role).IsEqualTo("reviewer"); + } + + [Test] + public async Task GetDefinition_is_404_for_a_definition_that_is_not_runnable() { + var definitions = Substitute.For(); + definitions.GetRunnableDescribedAsync("gone", Arg.Any()) + .ThrowsAsync(new FlowDefinitionNotRunnableException("Flow definition 'gone' is not available.")); + + var result = await Handlers(Substitute.For(), definitions: definitions) + .GetDefinition("gone", NewHttpContext(githubId: "42"), CancellationToken.None); + + var problem = result.Result as ProblemHttpResult; + await Assert.That(problem).IsNotNull(); + await Assert.That(problem!.StatusCode).IsEqualTo(StatusCodes.Status404NotFound); + await Assert.That(problem.ProblemDetails.Detail).Contains("not available"); + } + + [Test] + public async Task GetDefinition_returns_coded_409_when_the_catalog_has_not_projected() { + var definitions = Substitute.For(); + definitions.GetRunnableDescribedAsync(Arg.Any(), Arg.Any()) + .ThrowsAsync(new FlowCatalogNotReadyException("catching up")); + + var result = await Handlers(Substitute.For(), definitions: definitions) + .GetDefinition("code-review", NewHttpContext(githubId: "42"), CancellationToken.None); + + var refusal = result.Result as JsonHttpResult; + await Assert.That(refusal!.StatusCode).IsEqualTo(StatusCodes.Status409Conflict); + await Assert.That(refusal.Value!.Error).IsEqualTo(FlowErrorCodes.ServerCatchingUp); + } + + [Test] + public async Task GetDefinition_Unauthorized_WhenNoUser() { + var definitions = Substitute.For(); + + var result = await Handlers(Substitute.For(), definitions: definitions) + .GetDefinition("code-review", NewHttpContext(githubId: null), CancellationToken.None); + + await Assert.That(result.Result).IsTypeOf(); + await definitions.DidNotReceive().GetRunnableDescribedAsync(Arg.Any(), Arg.Any()); + } +``` + +Update the two existing `ListDefinitions_*` tests that stub `ListRunnableAsync`: stub `ListRunnableDescribedAsync` instead, wrapping each definition as `new DescribedFlowDefinition(def, null)`. Also change the `DidNotReceive()` call in `ListDefinitions_Unauthorized_WhenNoUser`. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Server.Tests.Flows/Capacitor.Server.Tests.Flows.csproj -- --treenode-filter "/*/*/FlowEndpointsTests/*"` +Expected: compile errors (`Offer`, `FlowDefinitionDetail` and `GetDefinition` are missing). + +- [ ] **Step 3: Implement.** + +`RunnableFlowDefinition.cs`. Leave the existing `` docs as they are; the new parameters have defaults so other constructions compile. + +```csharp +public sealed record RunnableFlowDefinition( + string Id, + int Version, + string? Description, + bool IsSingleParticipant, + IReadOnlyList Participants, + string Offer = "on_request", + string? WhenToUse = null); +``` + +`FlowDefinitionDetail.cs`: + +```csharp +namespace Capacitor.Api.Public.Abstractions.Flows; + +/// One runnable definition as a driver reads it before starting: the listing entry plus the guide for +/// driving it. Never the participants' prompts or the MCP allowlist. +public sealed record FlowDefinitionDetail( + string Id, + int Version, + string? Description, + bool IsSingleParticipant, + IReadOnlyList Participants, + string Offer, + string? WhenToUse, + string? DriverGuide); +``` + +In `FlowEndpointHandlers.cs`, replace the body of `ListDefinitions`' lambda and the `Describe` helper, and add `GetDefinition`: + +```csharp + return await RunFlowOpAsync(ctx, async () => { + var runnable = await definitions.ListRunnableDescribedAsync(ct); + + return new FlowDefinitionsResponse([..runnable.Select(Describe)]); + }); + } + + /// A definition that cannot start answers 404 rather than the shared mapping's 400, so a driver can + /// tell "no such flow" from a malformed request. + public async Task, JsonHttpResult, ProblemHttpResult, UnauthorizedHttpResult>> GetDefinition( + string definitionId, + HttpContext ctx, + CancellationToken ct + ) { + if (ctx.User.GetUserId() is null) return TypedResults.Unauthorized(); + + return await RunFlowOpAsync(ctx, async () => { + DescribedFlowDefinition described; + try { + described = await definitions.GetRunnableDescribedAsync(definitionId, ct); + } catch (FlowDefinitionNotRunnableException ex) { + throw new FlowNotFoundException(ex.Message); + } + + var (definition, guidance) = (described.Definition, described.Guidance); + + return new FlowDefinitionDetail( + definition.Id, definition.Version, definition.Description, definition.IsSingleParticipant, + Participants(definition), guidance?.Offer ?? FlowGuidanceOffer.OnRequest, guidance?.WhenToUse, guidance?.DriverGuide); + }); + } + + static RunnableFlowDefinition Describe(DescribedFlowDefinition described) => + new( + described.Definition.Id, + described.Definition.Version, + described.Definition.Description, + described.Definition.IsSingleParticipant, + Participants(described.Definition), + described.Guidance?.Offer ?? FlowGuidanceOffer.OnRequest, + described.Guidance?.WhenToUse); + + static IReadOnlyList Participants(FlowDefinition definition) => + [..definition.Participants.Select(p => new RunnableFlowParticipant(p.Name, p.Vendor, p.Model))]; +``` + +`FlowCatalogNotReadyException` is not a `FlowDefinitionNotRunnableException`, so it escapes the inner catch and reaches `RunFlowOpAsync`'s existing 409 arm. `FlowNotFoundException` maps to 404 there. If the old `Describe(FlowDefinition)` overload has no remaining caller, delete it. + +In `FlowEndpoints.cs`, directly after the `/definitions` mapping: + +```csharp + flows.MapGet("/definitions/{definitionId}", (string definitionId, HttpContext ctx, FlowEndpointHandlers h, CancellationToken ct) + => h.GetDefinition(definitionId, ctx, ct)) + .ProducesProblems(StatusCodes.Status404NotFound) + .ProducesJson(StatusCodes.Status409Conflict); +``` + +In `ServerJsonContext.cs`, add these to the `/api/flows` list: + +```csharp +[JsonSerializable(typeof(FlowDefinitionsResponse))] +[JsonSerializable(typeof(FlowDefinitionDetail))] +``` + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the Step 2 command. Expected: all `FlowEndpointsTests` pass. + +- [ ] **Step 5: Regenerate the OpenAPI snapshots.** + +Run: `dotnet run --project test/Capacitor.Server.Tests.Diagnostics/Capacitor.Server.Tests.Diagnostics.csproj -- --treenode-filter "/*/*/SchemaSnapshotTests/Update_committed_schemas"` + +Then run the same project without the filter. Expected: it passes, and `git diff test/Capacitor.Server.Tests.Diagnostics` shows exactly three additions: +- `offer` and `when_to_use` on `Flows_RunnableFlowDefinition` +- the new `/api/flows/definitions/{definitionId}` path +- `Flows_FlowDefinitionDetail` + +- [ ] **Step 6: Commit.** + +```bash +git add src/Capacitor.Api.Public.Abstractions/Flows/ src/Capacitor.Api.Public/Flows/ src/Capacitor.Server/ServerJsonContext.cs test/Capacitor.Server.Tests.Flows/FlowEndpointsTests.cs test/Capacitor.Server.Tests.Diagnostics/OpenApi/schemas/ +git commit -m "Serve flow guidance on the definitions listing and by id" +``` + +--- + +### Task 5: Admin editor schema + +**Files:** +- Modify: `src/Capacitor.Ui/wwwroot/flow-definition.schema.json` +- Test: `test/Capacitor.Source.Tests/Flows/FlowSchemaParityTests.cs` + +**Interfaces:** +- Consumes: the built-in YAML (Task 2) and `FlowDefinitionYamlParser.MaxWhenToUseLength` / `MaxDriverGuideLength` (Task 1). + +- [ ] **Step 1: Write the failing test.** Append to `FlowSchemaParityTests`. If `test/Capacitor.Source.Tests` has no YamlDotNet reference, add `` to its csproj; the server uses central package management, so no version is needed. + +```csharp + static string BuiltInPath(string leaf) => + Path.Combine(Path.GetDirectoryName(Path.GetDirectoryName(Path.GetDirectoryName(SchemaPath())!)!)!, + "Capacitor.Server.Services", "Flows", "Catalog", "BuiltIns", leaf); + + /// The editor validates against this schema, so a shipped built-in it rejects would show an operator red + /// squiggles on a definition the server accepts. Walks every mapping key and every required list. + [Test] + [Arguments("spec-review.yaml")] + [Arguments("code-review.yaml")] + public async Task Schema_accepts_every_key_of_each_built_in(string leaf) { + using var schema = JsonDocument.Parse(await File.ReadAllTextAsync(SchemaPath())); + var yaml = new YamlDotNet.RepresentationModel.YamlStream(); + yaml.Load(new StringReader(await File.ReadAllTextAsync(BuiltInPath(leaf)))); + + var problems = new List(); + Check((YamlDotNet.RepresentationModel.YamlMappingNode)yaml.Documents[0].RootNode, schema.RootElement, "", problems); + + await Assert.That(problems).IsEmpty(); + } + + static void Check(YamlDotNet.RepresentationModel.YamlMappingNode node, JsonElement schema, string path, List problems) { + if (schema.TryGetProperty("required", out var required)) + foreach (var key in required.EnumerateArray().Select(r => r.GetString()!)) + if (!node.Children.Keys.Any(k => k.ToString() == key)) + problems.Add($"{path}{key}: required by the schema, absent in the YAML"); + + foreach (var (keyNode, value) in node.Children) { + var key = keyNode.ToString(); + JsonElement child = default; + var known = schema.TryGetProperty("properties", out var props) && props.TryGetProperty(key, out child); + if (!known && schema.TryGetProperty("patternProperties", out var patterns)) + foreach (var p in patterns.EnumerateObject()) + if (System.Text.RegularExpressions.Regex.IsMatch(key, p.Name)) { child = p.Value; known = true; break; } + + if (!known) { problems.Add($"{path}{key}: not in the schema"); continue; } + if (value is YamlDotNet.RepresentationModel.YamlMappingNode mapping) Check(mapping, child, $"{path}{key}.", problems); + } + } + + [Test] + public async Task Schema_guidance_caps_match_the_parser() { + using var doc = JsonDocument.Parse(await File.ReadAllTextAsync(SchemaPath())); + var guidance = doc.RootElement.GetProperty("properties").GetProperty("guidance").GetProperty("properties"); + + await Assert.That(guidance.GetProperty("when_to_use").GetProperty("maxLength").GetInt32()).IsEqualTo(300); + await Assert.That(guidance.GetProperty("driver_guide").GetProperty("maxLength").GetInt32()).IsEqualTo(8192); + await Assert.That(guidance.GetProperty("offer").GetProperty("enum").EnumerateArray().Select(e => e.GetString())) + .IsEquivalentTo(new[] { "proactive", "on_request" }); + } +``` + +If `SchemaPath()`'s directory walk does not land on `src/` after three parents, compute the repository root the way `SchemaPath()` does (walk up to `Kurrent.Capacitor.slnx`) and join from there instead. The 300 and 8192 here are deliberate literals: Source.Tests may not reference Services, and the parser constants are pinned by Task 1's tests. + +- [ ] **Step 2: Run the tests and confirm they fail.** + +Run: `dotnet run --project test/Capacitor.Source.Tests/Capacitor.Source.Tests.csproj -- --treenode-filter "/*/*/FlowSchemaParityTests/*"` +Expected: FAIL. +- `reviewer.vendor` and `reviewer.model` are required but absent. +- `rounds.initial_prompt_context_only`, `rounds.follow_up_prompt_context_only` and `guidance` are not in the schema. +- `guidance` has no caps. + +- [ ] **Step 3: Implement.** Edit `flow-definition.schema.json`: + +1. In the participant schema (line 18), `"required": ["vendor", "model", "rounds"]` → `"required": ["rounds"]`. +2. In `reviewer` (line 48), delete `"required": ["vendor", "model"],`. +3. In both `rounds` objects, after `"follow_up_prompt": { "type": "string" },`, add: + ```json + "initial_prompt_context_only": { "type": "string", "description": "Initial prompt when the participant has no readable workspace." }, + "follow_up_prompt_context_only": { "type": "string", "description": "Follow-up prompt when the participant has no readable workspace." }, + ``` +4. After the `mcp` property (line 90), add: + ```json + , + "guidance": { + "type": "object", + "additionalProperties": false, + "description": "What a driving agent is told about this flow.", + "properties": { + "offer": { "type": "string", "enum": ["proactive", "on_request"], "description": "proactive: agents may offer it unprompted. Default on_request." }, + "when_to_use": { "type": "string", "maxLength": 300, "description": "When to offer it. Required when offer is proactive." }, + "driver_guide": { "type": "string", "maxLength": 8192, "description": "Markdown guide for driving the flow." } + }, + "if": { "properties": { "offer": { "const": "proactive" } }, "required": ["offer"] }, + "then": { "required": ["when_to_use"] } + } + ``` + Place the comma so the JSON stays valid; `Schema_file_is_well_formed_json` checks it. + +- [ ] **Step 4: Run the tests and confirm they pass.** + +Run the Step 2 command. Expected: all `FlowSchemaParityTests` pass. + +- [ ] **Step 5: Commit.** + +```bash +git add src/Capacitor.Ui/wwwroot/flow-definition.schema.json test/Capacitor.Source.Tests/ +git commit -m "Accept guidance and context-only prompts in the flow editor schema" +``` + +--- + +### Task 6: Verification + +- [ ] **Step 1: Build the solution.** `dotnet build Kurrent.Capacitor.slnx`. Expected: 0 warnings and 0 errors in the touched projects. +- [ ] **Step 2: Run the source scan.** `dotnet run --project test/Capacitor.Source.Tests/Capacitor.Source.Tests.csproj`. Expected: PASS. +- [ ] **Step 3: Run the flow suites.** Run each of the following: + - `dotnet run --project test/Capacitor.Server.Tests.Flows/Capacitor.Server.Tests.Flows.csproj` + - `dotnet run --project test/Capacitor.Server.Services.Tests/Capacitor.Server.Services.Tests.csproj` + - `dotnet run --project test/Capacitor.Server.Tests.Diagnostics/Capacitor.Server.Tests.Diagnostics.csproj` + + Expected: all pass. +- [ ] **Step 4: Check the editor bundle.** Run `dotnet build src/Capacitor.Ui` so the NpmBuild target rebundles `flow-monaco-bundle.js` with the schema. Commit the bundle if it is tracked and changed. From a35bdfc7ccaf049de9693814c94f322b3f3ed2e1 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 13:51:33 +0200 Subject: [PATCH 04/16] Show when to use each flow in list_flow_definitions Co-Authored-By: Claude Haiku 4.5 --- src/Capacitor.Cli/Commands/McpFlowsServer.cs | 6 ++++ .../McpFlowsServerFlowDefinitionsTests.cs | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/Capacitor.Cli/Commands/McpFlowsServer.cs b/src/Capacitor.Cli/Commands/McpFlowsServer.cs index d3beeb50f1..c08b14095f 100644 --- a/src/Capacitor.Cli/Commands/McpFlowsServer.cs +++ b/src/Capacitor.Cli/Commands/McpFlowsServer.cs @@ -1572,6 +1572,11 @@ record PollResult(string Payload, bool IsError); if (Str(definition, "description") is { Length: > 0 } description) sb.AppendLine($" {description.ReplaceLineEndings(" ").Trim()}"); + if (Str(definition, "when_to_use") is { Length: > 0 } whenToUse) + sb.AppendLine(Str(definition, "offer") == "proactive" + ? $" when to use (offer proactively): {whenToUse.ReplaceLineEndings(" ").Trim()}" + : $" when to use (on request): {whenToUse.ReplaceLineEndings(" ").Trim()}"); + foreach (var participant in participants.OfType()) { var role = Str(participant, "role") ?? "?"; var vendor = Str(participant, "vendor") ?? "vendor unset (the request or your saved flows.reviewer_vendor preference decides)"; @@ -2575,6 +2580,7 @@ internal static McpTool[] BuildToolsList() => [ "Call it before start_flow whenever the user has not named a definition, or named one you have not seen listed. " + "Read-only and side-effect-free: this does NOT start anything. " + "Each entry gives the id to pass as definition_id, its version and description, whether it is single-participant (start_flow runs round 1 and accepts vendor/model overrides) or multi-participant (start_flow is round-less; address each role with send_to_participant), and per participant its role, authored vendor (unset means the request or your saved preference decides) and model. " + + "A definition may also say when to use it, and whether you may offer it unprompted (offer proactively) or only run it when asked (on request). Before starting one, call get_flow_definition for its guide. " + "Disabled or deleted definitions are not listed, and start_flow refuses them. " + "A server_catching_up error means the catalog is temporarily unreadable — retry shortly rather than treating the list as empty; an empty list is authoritative.", new("object", new(), []), diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionsTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionsTests.cs index 3cea82b530..482f7c4923 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionsTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionsTests.cs @@ -164,4 +164,40 @@ public async Task The_tool_is_read_only_and_says_when_to_call_it() { await Assert.That(tool.Description).Contains("before start_flow"); await Assert.That(tool.Description).Contains("does NOT start"); } + + [Test] + public async Task A_proactive_definition_says_when_to_offer_it() { + var text = McpFlowsServer.FormatFlowDefinitions(""" + {"definitions":[{"id":"code-review","version":4,"is_single_participant":true, + "participants":[{"role":"reviewer","model":"default"}], + "offer":"proactive","when_to_use":"After a change\nis complete."}]} + """)!; + + await Assert.That(text).Contains(" when to use (offer proactively): After a change is complete."); + } + + [Test] + public async Task An_on_request_definition_shows_its_when_to_use_without_inviting_an_offer() { + var text = McpFlowsServer.FormatFlowDefinitions(""" + {"definitions":[{"id":"triage","participants":[{"role":"reviewer","model":"default"}], + "offer":"on_request","when_to_use":"When asked to triage."}]} + """)!; + + await Assert.That(text).Contains(" when to use (on request): When asked to triage."); + await Assert.That(text).DoesNotContain("offer proactively"); + } + + [Test] + public async Task A_server_without_guidance_fields_renders_as_before() { + var text = McpFlowsServer.FormatFlowDefinitions(TwoDefinitions)!; + + await Assert.That(text).DoesNotContain("when to use"); + } + + [Test] + public async Task The_listing_points_to_get_flow_definition() { + var tool = McpFlowsServer.BuildToolsList().Single(t => t.Name == "list_flow_definitions"); + + await Assert.That(tool.Description).Contains("get_flow_definition"); + } } From e41539039392d8b11013b191af3bb73d7262c200 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 14:13:48 +0200 Subject: [PATCH 05/16] Add get_flow_definition to read a flow's driver guide Co-Authored-By: Claude Opus 5.5 --- src/Capacitor.Cli/Commands/McpFlowsServer.cs | 79 ++++++++++ .../McpFlowsServerTests.cs | 3 +- ...McpFlowsServerFlowDefinitionDetailTests.cs | 135 ++++++++++++++++++ .../Commands/McpToolAnnotationsTests.cs | 1 + 4 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionDetailTests.cs diff --git a/src/Capacitor.Cli/Commands/McpFlowsServer.cs b/src/Capacitor.Cli/Commands/McpFlowsServer.cs index c08b14095f..8dd2eb0f2a 100644 --- a/src/Capacitor.Cli/Commands/McpFlowsServer.cs +++ b/src/Capacitor.Cli/Commands/McpFlowsServer.cs @@ -470,6 +470,35 @@ internal async Task HandleToolCallAsync( : BuildToolResult(id, "Error: unreadable flow definition list from GET /api/flows/definitions.", isError: true); } + if (toolName is "get_flow_definition") { + var definitionId = arguments?["definition_id"] is JsonValue dv && dv.TryGetValue(out string? s) ? s?.Trim() : null; + if (string.IsNullOrEmpty(definitionId)) + return BuildToolResult(id, "Error: definition_id is required — pass an id from list_flow_definitions.", isError: true); + + var lookup = await GetBoundedAsync(client, $"{apiRoot}/api/flows/definitions/{Uri.EscapeDataString(definitionId)}", clock); + if (lookup.Response is null) + return BuildToolResult(id, $"Error: reading flow definition '{definitionId}' {lookup.How}; nothing was started — retry the call.", isError: true); + + using var detailResp = lookup.Response; + var detailBody = await detailResp.Content.ReadAsStringAsync(); + + if (detailResp.StatusCode == HttpStatusCode.Unauthorized) + return BuildToolResult(id, await AuthRejectionNotice.ForPersistentUnauthorizedAsync(store, profiles.Name, apiRoot, time), isError: true); + + // A server that serves the route answers an unknown id with a problem body; a routing 404 has none. + if (detailResp.StatusCode == HttpStatusCode.NotFound) + return ProblemDetail(detailBody) is { } detail + ? BuildToolResult(id, $"Error: {detail} Call list_flow_definitions for the ids this server can start.", isError: true) + : BuildToolResult(id, ServerPublishesNoGuides); + + if (!detailResp.IsSuccessStatusCode) + return BuildToolResult(id, FormatFlowStartError((int)detailResp.StatusCode, detailBody, wasDynamicStart: false), isError: true); + + return FormatFlowDefinitionDetail(detailBody) is { } rendered + ? BuildToolResult(id, rendered) + : BuildToolResult(id, $"Error: unreadable flow definition from GET /api/flows/definitions/{definitionId}.", isError: true); + } + if (toolName is StartAgentOptionsTool.Name) { var lookup = await GetBoundedAsync(client, apiRoot + StartAgentOptionsTool.Route, clock); if (lookup.Response is null) @@ -1595,6 +1624,40 @@ record PollResult(string Payload, bool IsError); static bool? Bool(JsonObject o, string key) => o[key] is JsonValue v && v.TryGetValue(out bool b) ? b : null; } + internal const string ServerPublishesNoGuides = + "This server does not publish flow guides (GET /api/flows/definitions/{id} is not available). " + + "Use the definition's description from list_flow_definitions and the agent-flows skill's rules."; + + /// Renders GET /api/flows/definitions/{id}: the listing entry, then the authored driver guide verbatim. + /// Null when the body is not a definition. + internal static string? FormatFlowDefinitionDetail(string body) { + JsonNode? root; + try { root = JsonNode.Parse(body); } catch (JsonException) { return null; } + + if (root is not JsonObject definition || definition["id"] is not JsonValue) return null; + + var listing = FormatFlowDefinitions(new JsonObject { ["definitions"] = new JsonArray(definition.DeepClone()) }.ToJsonString()); + if (listing is null) return null; + + // Keep the entry lines only: drop the "flow_definitions (1):" header and the closing hint. + var lines = listing.Split('\n'); + var entry = string.Join('\n', lines[1..^1]).TrimEnd(); + + var guide = definition["driver_guide"] is JsonValue g && g.TryGetValue(out string? text) && !string.IsNullOrWhiteSpace(text) + ? $"Driver guide:\n{text.Trim()}" + : "This definition publishes no driver guide: follow its description and the agent-flows skill's rules."; + + return $"{entry}\n\n{guide}"; + } + + static string? ProblemDetail(string body) { + try { + return JsonNode.Parse(body) is JsonObject o && o["detail"] is JsonValue v && v.TryGetValue(out string? d) && !string.IsNullOrWhiteSpace(d) ? d : null; + } catch (JsonException) { + return null; + } + } + /// Renders an exhausted settlement elapsed deadline as tool-error text, in the same /// "Error (code): message" shape uses for a coded rejection — /// the caller sees the real server code it kept hitting, plus how hard the CLI tried, plus the @@ -2586,6 +2649,22 @@ internal static McpTool[] BuildToolsList() => [ new("object", new(), []), McpToolAnnotations.Read ), + new( + "get_flow_definition", + "Read one flow definition's guide before start_flow: what context to submit, how to iterate on its results and when to close. " + + "Read-only and side-effect-free: this does NOT start anything. " + + "Returns the definition's listing entry (participants, when to use it) followed by its authored driver guide. " + + "An unknown, disabled or deleted id is an error — call list_flow_definitions for the ids this server can start. " + + "A server that does not publish guides says so; then rely on the definition's description.", + new( + "object", + new() { + ["definition_id"] = new("string", "Definition id from list_flow_definitions, e.g. 'code-review'.") + }, + ["definition_id"] + ), + McpToolAnnotations.Read + ), StartAgentOptionsTool.Describe(), StartAgentTool.Describe() ]; diff --git a/test/Capacitor.Cli.Tests.Integration/McpFlowsServerTests.cs b/test/Capacitor.Cli.Tests.Integration/McpFlowsServerTests.cs index 5a51cc1f7c..9d8b1be2a7 100644 --- a/test/Capacitor.Cli.Tests.Integration/McpFlowsServerTests.cs +++ b/test/Capacitor.Cli.Tests.Integration/McpFlowsServerTests.cs @@ -305,7 +305,7 @@ public async Task Tools_list_returns_every_flow_tool() { var tools = response["result"]?["tools"]?.AsArray(); await Assert.That(tools).IsNotNull(); - await Assert.That(tools!.Count).IsEqualTo(12); + await Assert.That(tools!.Count).IsEqualTo(13); var names = tools.Select(t => t?["name"]?.GetValue()).ToHashSet(); await Assert.That(names.Contains("start_review_flow")).IsTrue(); @@ -318,6 +318,7 @@ public async Task Tools_list_returns_every_flow_tool() { await Assert.That(names.Contains("close_flow")).IsTrue(); await Assert.That(names.Contains("list_reviewer_vendors")).IsTrue(); await Assert.That(names.Contains("list_flow_definitions")).IsTrue(); + await Assert.That(names.Contains("get_flow_definition")).IsTrue(); await Assert.That(names.Contains("start_agent")).IsTrue(); await Assert.That(names.Contains("list_start_agent_options")).IsTrue(); } finally { diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionDetailTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionDetailTests.cs new file mode 100644 index 0000000000..1b1cadb637 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerFlowDefinitionDetailTests.cs @@ -0,0 +1,135 @@ +using System.Text.Json.Nodes; +using Capacitor.Cli.Commands; +using Capacitor.Cli.Core; +using Capacitor.Cli.PrDetection; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using WireMock.Server; + +namespace Capacitor.Cli.Tests.Unit.Commands; + +public class McpFlowsServerFlowDefinitionDetailTests { + [TempConfigRoot] public required TempConfigRoot Config { get; init; } + + McpFlowsServer Server() => + new(Config.Root, Resolutions.None(Config.Root), AuthFixtures.NewTokenStore(Config.Root), + new FixedCapacitorHttpClient(), NoTelemetry.Startup, router: new GitProviderRouter(), workdir: new WorkingDirectory(AppContext.BaseDirectory), time: TimeProvider.System); + + static JsonObject ToolCall(JsonObject arguments) => new() { + ["params"] = new JsonObject { ["name"] = "get_flow_definition", ["arguments"] = arguments } + }; + + static (string Text, bool IsError) Result(string response) { + var result = JsonNode.Parse(response)!["result"]!; + + return (result["content"]![0]!["text"]!.GetValue(), result["isError"]?.GetValue() ?? false); + } + + static WireMockServer Serving(string id, int status, string body, string contentType = "application/json") { + var server = WireMockServer.Start(); + server.Given(Request.Create().WithPath($"/api/flows/definitions/{id}").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(status).WithHeader("Content-Type", contentType).WithBody(body)); + + return server; + } + + async Task<(string Text, bool IsError)> CallAsync(WireMockServer server, string id) { + using var client = new HttpClient(); + + return Result(await Server().HandleToolCallAsync( + JsonNode.Parse("1")!, ToolCall(new JsonObject { ["definition_id"] = id }), client, server.Url!, cwd: "/r", repoRoot: "/r", repoInfo: null)); + } + + const string CodeReview = """ + {"id":"code-review","version":4,"description":"Review code changes.","is_single_participant":true, + "participants":[{"role":"reviewer","vendor":null,"model":"default"}], + "offer":"proactive","when_to_use":"After a change.","driver_guide":"## What to submit\nThe commit range."} + """; + + [Test] + public async Task Renders_the_guide_under_the_definition() { + using var server = Serving("code-review", 200, CodeReview); + + var (text, isError) = await CallAsync(server, "code-review"); + + await Assert.That(isError).IsFalse(); + await Assert.That(text).Contains("code-review (v4) — single participant"); + await Assert.That(text).Contains(" reviewer: vendor unset"); + await Assert.That(text).Contains("when to use (offer proactively): After a change."); + await Assert.That(text).Contains("Driver guide:\n## What to submit\nThe commit range."); + } + + [Test] + public async Task A_definition_without_a_guide_says_so() { + using var server = Serving("plain", 200, """{"id":"plain","participants":[{"role":"reviewer","model":"default"}],"offer":"on_request"}"""); + + var (text, isError) = await CallAsync(server, "plain"); + + await Assert.That(isError).IsFalse(); + await Assert.That(text).Contains("This definition publishes no driver guide"); + } + + [Test] + public async Task An_unknown_id_is_an_error_naming_the_listing() { + using var server = Serving("nope", 404, """{"status":404,"detail":"Flow definition 'nope' is not available."}""", "application/problem+json"); + + var (text, isError) = await CallAsync(server, "nope"); + + await Assert.That(isError).IsTrue(); + await Assert.That(text).Contains("Flow definition 'nope' is not available."); + await Assert.That(text).Contains("list_flow_definitions"); + } + + [Test] + public async Task An_older_server_without_the_route_is_reported_not_errored() { + using var server = Serving("code-review", 404, "", "text/plain"); + + var (text, isError) = await CallAsync(server, "code-review"); + + await Assert.That(isError).IsFalse(); + await Assert.That(text).IsEqualTo(McpFlowsServer.ServerPublishesNoGuides); + } + + [Test] + public async Task A_catalog_that_has_not_projected_is_a_retryable_refusal() { + using var server = Serving("code-review", 409, """{"error":"server_catching_up","message":"Flows are temporarily unavailable."}"""); + + var (text, isError) = await CallAsync(server, "code-review"); + + await Assert.That(isError).IsTrue(); + await Assert.That(text).Contains(McpFlowsServer.ServerCatchingUpGuidance); + } + + [Test] + public async Task A_missing_definition_id_is_rejected_without_a_request() { + using var server = Serving("x", 200, CodeReview); + using var client = new HttpClient(); + + var (text, isError) = Result(await Server().HandleToolCallAsync( + JsonNode.Parse("1")!, ToolCall(new JsonObject()), client, server.Url!, cwd: "/r", repoRoot: "/r", repoInfo: null)); + + await Assert.That(isError).IsTrue(); + await Assert.That(text).Contains("definition_id is required"); + await Assert.That(server.LogEntries.Count).IsEqualTo(0); + } + + [Test] + public async Task An_id_is_escaped_into_one_path_segment() { + using var server = WireMockServer.Start(); + using var client = new HttpClient(); + + await Server().HandleToolCallAsync( + JsonNode.Parse("1")!, ToolCall(new JsonObject { ["definition_id"] = "a/b" }), client, server.Url!, cwd: "/r", repoRoot: "/r", repoInfo: null); + + await Assert.That(server.LogEntries.Single().RequestMessage.Url).Contains("/api/flows/definitions/a%2Fb"); + } + + [Test] + public async Task The_tool_is_read_only_and_requires_definition_id() { + var tool = McpFlowsServer.BuildToolsList().Single(t => t.Name == "get_flow_definition"); + + await Assert.That(tool.Annotations).IsEqualTo(McpToolAnnotations.Read); + await Assert.That(tool.InputSchema.Required).IsEquivalentTo(new[] { "definition_id" }); + await Assert.That(tool.Description).Contains("before start_flow"); + } +} diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/McpToolAnnotationsTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/McpToolAnnotationsTests.cs index 5a9a86d725..78dbdba972 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/McpToolAnnotationsTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/McpToolAnnotationsTests.cs @@ -50,6 +50,7 @@ public async Task Reads_are_read_only_and_writes_say_what_they_do() { await Assert.That(Tool("kcap-memory", "search_memories").Annotations.ReadOnlyHint).IsTrue(); await Assert.That(Tool("kcap-flows", "list_reviewer_vendors").Annotations.ReadOnlyHint).IsTrue(); await Assert.That(Tool("kcap-flows", "list_flow_definitions").Annotations.ReadOnlyHint).IsTrue(); + await Assert.That(Tool("kcap-flows", "get_flow_definition").Annotations.ReadOnlyHint).IsTrue(); var declare = Tool("kcap-plans", "declare_plan_document").Annotations; await Assert.That(declare.ReadOnlyHint).IsFalse(); From 362092eac3766c223d518c97d530dcd44ba52cd2 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 14:26:38 +0200 Subject: [PATCH 06/16] Offer proactive catalogue flows in session-start context Co-Authored-By: Claude Opus 5.5 --- .../SessionStartCompositeContextProvider.cs | 91 ++++------ .../SessionStartFlowsLane.cs | 88 +++++++++ .../SessionStartMemoryContracts.cs | 11 +- .../SessionStartMemoryHookSupport.cs | 11 +- .../SessionStartMemoryOrchestrator.cs | 6 +- .../SessionStartMemory/FlowsLaneTests.cs | 170 ++++++++++++++++++ .../GuidelinesLaneAndCompositeTests.cs | 27 +-- .../SessionStartMemory/SessionStartStubs.cs | 26 +++ 8 files changed, 336 insertions(+), 94 deletions(-) create mode 100644 src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs create mode 100644 test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs create mode 100644 test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs index b162fe26da..3dbb9b9591 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs @@ -2,31 +2,19 @@ namespace Capacitor.Cli.SessionStartMemory; -/// -/// The eight non-Claude harnesses' SessionStart context provider: one -/// combined fragment carrying both the team-memory index and judge-fact -/// guidelines. Resolves the repo/machine scope ONCE and drives the memory lane -/// () and the -/// guidelines lane () -/// in parallel under one budget, then composes a single string that rides the -/// harness's existing memory delivery seam unchanged. -/// -/// Disposition is computed over ENABLED lanes only: any content ⇒ commit; -/// all empty ⇒ complete-without-context; no content and ≥1 retryable failure ⇒ -/// retry (with the max of the lanes' Retry-After hints). A disabled lane -/// contributes nothing and never blocks commit. -/// +/// The eight non-Claude harnesses' SessionStart context provider: one fragment from the memory, +/// guidelines and flows lanes, the memory marker first. Scope is resolved once and the enabled lanes run in +/// parallel under one budget; a disabled lane contributes nothing and never blocks commit. internal sealed class SessionStartCompositeContextProvider( ISessionStartMemoryScopeResolver scopeResolver, SessionStartMemoryContextProvider memory, SessionStartGuidelinesLane guidelines, + SessionStartFlowsLane flows, TimeProvider time, Action? diagnostic = null) : ISessionStartContextProvider { public async Task GetAsync(SessionStartMemoryContextRequest request) { - var memoryEnabled = !request.Disabled; - var guidelinesEnabled = !request.GuidelinesDisabled; - if (!memoryEnabled && !guidelinesEnabled) return SessionStartMemoryContextResult.Empty; + if (request.AllLanesDisabled) return SessionStartMemoryContextResult.Empty; if (request.Budget <= TimeSpan.Zero) return SessionStartMemoryContextResult.Retry; using var expiry = new CancellationTokenSource(request.Budget, time); @@ -40,18 +28,18 @@ public async Task GetAsync(SessionStartMemoryCo return SessionStartMemoryContextResult.Retry; } - // Start both enabled lanes before awaiting, so they run in parallel under the shared budget. - var memoryTask = memoryEnabled - ? RunLaneAsync(() => memory.FetchWithScopeAsync(scope, request, cts.Token)) - : null; - var guidelinesTask = guidelinesEnabled - ? RunLaneAsync(() => guidelines.FetchWithScopeAsync(scope, request, cts.Token)) - : null; + // Started before any await so the lanes share the budget in parallel. + var memoryTask = request.Disabled ? null : RunLaneAsync(() => memory.FetchWithScopeAsync(scope, request, cts.Token)); + var guidelinesTask = request.GuidelinesDisabled ? null : RunLaneAsync(() => guidelines.FetchWithScopeAsync(scope, request, cts.Token)); + var flowsTask = request.FlowsDisabled ? null : RunLaneAsync(() => flows.FetchAsync(request, cts.Token)); - var memoryResult = memoryTask is null ? null : await memoryTask; - var guidelinesResult = guidelinesTask is null ? null : await guidelinesTask; + var memoryResult = memoryTask is null ? null : await memoryTask; + SessionStartMemoryContextResult?[] others = [ + guidelinesTask is null ? null : await guidelinesTask, + flowsTask is null ? null : await flowsTask + ]; - return Combine(memoryResult, guidelinesResult); + return Combine(memoryResult, others); } async Task RunLaneAsync(Func> lane) { @@ -63,44 +51,31 @@ async Task RunLaneAsync(Func().ToList(); - if (memoryFragment is not null || guidelinesFragment is not null) - return new SessionStartMemoryContextResult( - SessionStartMemoryDisposition.Ready, Compose(memoryFragment, guidelinesFragment)); + if (memoryFragment is not null || otherFragments.Count > 0) + return new SessionStartMemoryContextResult(SessionStartMemoryDisposition.Ready, Compose(memoryFragment, otherFragments)); - // No content. If every enabled lane was empty (not failed), the session genuinely has nothing - // to inject → complete the lease. Otherwise ≥1 lane failed retryably → hold for a later attempt. - var anyRetry = memoryResult?.Disposition == SessionStartMemoryDisposition.RetryableFailure - || guidelinesResult?.Disposition == SessionStartMemoryDisposition.RetryableFailure; - if (!anyRetry) return SessionStartMemoryContextResult.Empty; + SessionStartMemoryContextResult?[] all = [memoryResult, ..others]; + var retries = all.Where(r => r?.Disposition == SessionStartMemoryDisposition.RetryableFailure).ToList(); + if (retries.Count == 0) return SessionStartMemoryContextResult.Empty; return new SessionStartMemoryContextResult( - SessionStartMemoryDisposition.RetryableFailure, RetryAfter: MaxRetryAfter(memoryResult, guidelinesResult)); - } + SessionStartMemoryDisposition.RetryableFailure, RetryAfter: retries.Max(r => r!.RetryAfter)); - /// - /// One fragment, marker-first. The memory fragment already opens with the - /// shared kcap-memory-index marker; the guidelines fragment is - /// marker-less, so in the guidelines-only case the marker is prepended here - /// — Pi/OpenCode capture stdout only when it OPENS with that marker. - /// - static string Compose(string? memoryFragment, string? guidelinesFragment) { - if (memoryFragment is not null && guidelinesFragment is not null) - return memoryFragment + "\n\n" + guidelinesFragment; - if (memoryFragment is not null) return memoryFragment; - return MemoryIndexEmitter.FragmentMarker + "\n" + guidelinesFragment; + static string? Ready(SessionStartMemoryContextResult? r) => + r is { Disposition: SessionStartMemoryDisposition.Ready, Fragment: { } f } ? f : null; } - static TimeSpan? MaxRetryAfter(SessionStartMemoryContextResult? a, SessionStartMemoryContextResult? b) { - var x = a?.RetryAfter; - var y = b?.RetryAfter; - if (x is null) return y; - if (y is null) return x; - return x.Value >= y.Value ? x : y; + /// Marker first: Pi and OpenCode capture stdout only when it opens with it, and only the memory + /// fragment carries it. + static string Compose(string? memoryFragment, IReadOnlyList others) { + var rest = string.Join("\n\n", others); + if (memoryFragment is null) return MemoryIndexEmitter.FragmentMarker + "\n" + rest; + + return others.Count == 0 ? memoryFragment : memoryFragment + "\n\n" + rest; } static bool IsFailOpen(Exception ex) => diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs new file mode 100644 index 0000000000..3781bbd67c --- /dev/null +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs @@ -0,0 +1,88 @@ +using System.Net; +using System.Text; +using System.Text.Json.Nodes; +using Capacitor.Cli.Core.WorkItems; + +namespace Capacitor.Cli.SessionStartMemory; + +/// The flows lane of the composite SessionStart context: the catalogue flows an operator marked +/// offer: proactive, each with when to offer it. Marker-less, like the guidelines lane. +internal sealed class SessionStartFlowsLane(Func> client, TimeProvider time) { + public const int MaxDescribed = 10; + public const int MaxFragmentChars = 2048; + const int WhenToUseCap = 300; + const int IdCap = 64; + + public const string Header = "Flows you may offer (ask first; never start unprompted):"; + public const string Footer = "Before starting one, call get_flow_definition() and follow its guide."; + + public async Task FetchAsync(SessionStartMemoryContextRequest request, CancellationToken ct) { + var outcome = await SessionStartContextFetch.FetchAsync( + await client(ct), request.BaseUrl.TrimEnd('/') + "/api/flows/definitions", time, ct); + + // A server older than the listing has nothing to offer, now or on a retry. + if (outcome.Status == HttpStatusCode.NotFound) return SessionStartMemoryContextResult.Empty; + if (outcome.Body is null) + return new SessionStartMemoryContextResult(SessionStartMemoryDisposition.RetryableFailure, RetryAfter: outcome.RetryAfter); + + return BuildFragment(JsonNode.Parse(outcome.Body)) is { } fragment + ? new SessionStartMemoryContextResult(SessionStartMemoryDisposition.Ready, fragment) + : SessionStartMemoryContextResult.Empty; + } + + public static string? BuildFragment(JsonNode? root) { + if (root is not JsonObject { } obj || obj["definitions"] is not JsonArray definitions) return null; + + var offered = definitions.OfType() + .Where(d => Str(d, "offer") == "proactive") + .Select(d => (Id: NextWorkUntrustedText.Render(Str(d, "id"), IdCap), When: NextWorkUntrustedText.Render(Str(d, "when_to_use"), WhenToUseCap))) + .Where(f => f.Id.Length > 0) + .ToList(); + if (offered.Count == 0) return null; + + // Described in order until the count or the room runs out; everything after falls back to its id, so the + // server's id order survives. The overflow line has its own fixed reserve, so the block never passes the cap. + var room = MaxFragmentChars - Header.Length - Footer.Length - 2 - OverflowReserve; + var described = new StringBuilder(); + var idsOnly = new List(); + var count = 0; + + foreach (var (id, when) in offered) { + var line = when.Length > 0 ? $"- {id}: {when}\n" : $"- {id}\n"; + if (idsOnly.Count == 0 && count < MaxDescribed && described.Length + line.Length <= room) { + described.Append(line); + count++; + } else { + idsOnly.Add(id); + } + } + + var sb = new StringBuilder(); + sb.Append(Header).Append('\n').Append(described); + if (idsOnly.Count > 0) sb.Append(Overflow(idsOnly)).Append('\n'); + sb.Append(Footer); + + return sb.ToString(); + + static string Overflow(List ids) { + var line = new StringBuilder("Also offerable: "); + var shown = 0; + foreach (var id in ids) { + var piece = shown == 0 ? id : ", " + id; + if (line.Length + piece.Length > OverflowReserve - OverflowTail.Length - MoreSuffixReserve) break; + line.Append(piece); + shown++; + } + + if (shown < ids.Count) line.Append($" and {ids.Count - shown} more").Append(OverflowTail); + + return line.ToString(); + } + + static string? Str(JsonObject o, string key) => o[key] is JsonValue v && v.TryGetValue(out string? s) ? s : null; + } + + const int OverflowReserve = 400; + const int MoreSuffixReserve = 16; + const string OverflowTail = " (see list_flow_definitions)"; +} diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs index 21b87a822d..0df3032e3d 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs @@ -44,17 +44,18 @@ internal sealed record SessionStartMemoryIndexResponse( [property: JsonPropertyName("entries")] SessionStartMemoryEntry[]? Entries, [property: JsonPropertyName("projects")] SessionStartMemoryProject[]? Projects); +/// Each lane has its own opt-out. The guidelines and flows flags default to off so a memory-only +/// construction stays memory-only. internal sealed record SessionStartMemoryContextRequest( string BaseUrl, string? Cwd, bool Disabled, TimeSpan Budget, CancellationToken CancellationToken, - // the guidelines lane's opt-out (disable_session_guidelines), independent of the - // memory lane's Disabled (disable_memory_index). Additive with a default so the Claude - // memory-only construction — which never runs the guidelines lane — compiles untouched and - // stays guidelines-off. The eight non-Claude adapters set it explicitly from activeProfile. - bool GuidelinesDisabled = true); + bool GuidelinesDisabled = true, + bool FlowsDisabled = true) { + public bool AllLanesDisabled => Disabled && GuidelinesDisabled && FlowsDisabled; +} internal sealed record SessionStartMemoryContextResult( SessionStartMemoryDisposition Disposition, diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs index e0bab6b533..59328bdfa4 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs @@ -1,5 +1,7 @@ using Capacitor.Cli.Commands; using Capacitor.Cli.Core; +using Capacitor.Cli.Core.Harness; +using Capacitor.Cli.Core.Mcp; using Capacitor.Cli.PrDetection; namespace Capacitor.Cli.SessionStartMemory; @@ -11,7 +13,7 @@ namespace Capacitor.Cli.SessionStartMemory; /// internal static class SessionStartMemoryHookSupport { /// - /// Builds the combined memory + guidelines SessionStart context provider. Both lanes draw from one + /// Builds the combined memory, guidelines and flows SessionStart context provider. Both lanes draw from one /// authenticated client accessor and the composite resolves the repo/machine scope ONCE for both. /// Which lanes actually run is decided per request via /// (memory) and its @@ -38,9 +40,14 @@ public static ISessionStartContextProvider CompositeProvider( var memory = new SessionStartMemoryContextProvider(resolver, client, time); var guidelines = new SessionStartGuidelinesLane(client, time); - return new SessionStartCompositeContextProvider(resolver, memory, guidelines, time); + var flows = new SessionStartFlowsLane(client, time); + return new SessionStartCompositeContextProvider(resolver, memory, guidelines, flows, time); } + /// Offering a flow the agent cannot start is worse than not offering it. + public static bool FlowsLaneDisabled(HarnessId harness, HarnessRegistry harnesses, string? codexConfigPath = null) => + !McpServerNudgeAvailability.IsRegisteredFor(harness, harnesses, KcapMcpServers.FlowsServerName, codexConfigPath); + /// /// Awaits an in-flight fragment fetch under the budget remaining AT THIS INSTANT — never the /// budget it was started with. On expiry the fetch is abandoned rather than cancelled mid-flight diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs index b9a6567ef8..69bcceaf9f 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs @@ -23,10 +23,8 @@ internal sealed class SessionStartMemoryOrchestrator( public async Task GetFragmentAsync(SessionMemoryLifecycle lifecycle, SessionStartMemoryContextRequest request, Func>? commitGate = null) { - // Both lanes disabled ⇒ no lease is spent, so a flag flipped on mid-session can still - // inject on a later callback (the disabled-lane disposition rule). A single disabled lane does NOT short-circuit - // here — the composite provider runs the enabled lane and contributes its content. - if (request.Disabled && request.GuidelinesDisabled) return null; + // Every lane off spends no lease, so a lane switched on mid-session can still inject on a later callback. + if (request.AllLanesDisabled) return null; var started = time.GetTimestamp(); TimeSpan Remaining() { diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs new file mode 100644 index 0000000000..c549646640 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs @@ -0,0 +1,170 @@ +using System.Net; +using System.Text.Json.Nodes; +using Capacitor.Cli.SessionStartMemory; +using Microsoft.Extensions.Time.Testing; + +namespace Capacitor.Cli.Tests.Unit.SessionStartMemory; + +public class FlowsLaneTests { + static Func> Lazy(HttpClient client) => _ => Task.FromResult(client); + + static SessionStartMemoryContextRequest Req(bool memory = false, bool guidelines = false, bool flows = true) => + new("https://example.test", "/repo", Disabled: !memory, TimeSpan.FromSeconds(1), CancellationToken.None, + GuidelinesDisabled: !guidelines, FlowsDisabled: !flows); + + static SessionStartFlowsLane Lane(HttpStatusCode status, string body) => + new(Lazy(new HttpClient(new Handler(status, body, null))), TimeProvider.System); + + const string Catalog = """ + {"definitions":[ + {"id":"code-review","offer":"proactive","when_to_use":"After a change is complete."}, + {"id":"spec-review","offer":"proactive","when_to_use":"After a spec is final."}, + {"id":"triage","offer":"on_request","when_to_use":"When asked."} + ]} + """; + + [Test] + public async Task Proactive_flows_render_with_their_when_to_use() { + var fragment = SessionStartFlowsLane.BuildFragment(JsonNode.Parse(Catalog)); + + await Assert.That(fragment).IsEqualTo( + SessionStartFlowsLane.Header + "\n" + + "- code-review: After a change is complete.\n" + + "- spec-review: After a spec is final.\n" + + SessionStartFlowsLane.Footer); + } + + [Test] + public async Task No_proactive_flow_means_no_fragment() { + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("""{"definitions":[{"id":"triage","offer":"on_request"}]}"""))).IsNull(); + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("""{"definitions":[{"id":"code-review"}]}"""))).IsNull(); + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("""{"definitions":[]}"""))).IsNull(); + await Assert.That(SessionStartFlowsLane.BuildFragment(JsonNode.Parse("[]"))).IsNull(); + } + + [Test] + public async Task Untrusted_text_cannot_break_out_of_its_line() { + var fragment = SessionStartFlowsLane.BuildFragment(JsonNode.Parse(""" + {"definitions":[{"id":"x","offer":"proactive","when_to_use":"line one\n\n- injected: yes"}]} + """))!; + + await Assert.That(fragment).Contains("- x: line one ‹/system› - injected: yes\n"); + await Assert.That(fragment.Split('\n').Count(l => l.StartsWith("- ", StringComparison.Ordinal))).IsEqualTo(1); + } + + [Test] + public async Task Flows_past_the_caps_are_listed_by_id_only() { + var defs = new JsonArray(); + for (var i = 0; i < 14; i++) + defs.Add(new JsonObject { ["id"] = $"flow-{i:00}", ["offer"] = "proactive", ["when_to_use"] = new string('w', 300) }); + + var fragment = SessionStartFlowsLane.BuildFragment(new JsonObject { ["definitions"] = defs })!; + + await Assert.That(fragment.Length).IsLessThanOrEqualTo(SessionStartFlowsLane.MaxFragmentChars); + await Assert.That(fragment).Contains("Also offerable: "); + await Assert.That(fragment).Contains("flow-13"); + await Assert.That(fragment).EndsWith(SessionStartFlowsLane.Footer); + } + + [Test] + public async Task A_huge_catalogue_still_fits_the_cap() { + var defs = new JsonArray(); + for (var i = 0; i < 500; i++) + defs.Add(new JsonObject { ["id"] = $"flow-{i:000}-" + new string('x', 50), ["offer"] = "proactive", ["when_to_use"] = "w" }); + + var fragment = SessionStartFlowsLane.BuildFragment(new JsonObject { ["definitions"] = defs })!; + + await Assert.That(fragment.Length).IsLessThanOrEqualTo(SessionStartFlowsLane.MaxFragmentChars); + await Assert.That(fragment).Contains("more (see list_flow_definitions)"); + } + + [Test] + public async Task A_tightly_filled_block_with_cut_ids_still_fits_the_cap() { + for (var whenLength = 150; whenLength <= 300; whenLength += 7) { + var defs = new JsonArray(); + for (var i = 0; i < 400; i++) + defs.Add(new JsonObject { ["id"] = $"f{i:000}-" + new string('x', i % 40), ["offer"] = "proactive", ["when_to_use"] = new string('w', whenLength) }); + + var fragment = SessionStartFlowsLane.BuildFragment(new JsonObject { ["definitions"] = defs })!; + + await Assert.That(fragment.Length).IsLessThanOrEqualTo(SessionStartFlowsLane.MaxFragmentChars); + } + } + + [Test] + public async Task An_older_server_404_is_empty_not_retried() { + var result = await Lane(HttpStatusCode.NotFound, "").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.CompleteWithoutContext); + } + + [Test] + public async Task A_server_error_is_retryable() { + var result = await Lane(HttpStatusCode.ServiceUnavailable, "").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.RetryableFailure); + } + + [Test] + public async Task A_listing_without_guidance_fields_is_empty() { + var result = await Lane(HttpStatusCode.OK, """{"definitions":[{"id":"code-review","version":3}]}""").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.CompleteWithoutContext); + } + + [Test] + public async Task The_lane_reads_the_definitions_listing() { + var handler = new Handler(HttpStatusCode.OK, Catalog, null); + var lane = new SessionStartFlowsLane(Lazy(new HttpClient(handler)), TimeProvider.System); + + var result = await lane.FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(new Uri(handler.Uri!).AbsolutePath).IsEqualTo("/api/flows/definitions"); + } + + [Test] + public async Task With_every_other_lane_off_the_composite_still_injects_flows() { + var composite = Composite(memory: (HttpStatusCode.OK, "[]"), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)); + + var result = await composite.GetAsync(Req(memory: false, guidelines: false, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(result.Fragment!).StartsWith(MemoryIndexEmitter.FragmentMarker + "\n" + SessionStartFlowsLane.Header); + } + + [Test] + public async Task A_failing_flows_lane_leaves_the_memory_fragment_untouched() { + const string memoryBody = """[{"memory_id":"m1","slug":"s","audience":"org","description":"d","kind":"preference"}]"""; + var withFlows = await Composite(memory: (HttpStatusCode.OK, memoryBody), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.InternalServerError, "")) + .GetAsync(Req(memory: true, flows: true)); + var withoutFlows = await Composite(memory: (HttpStatusCode.OK, memoryBody), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, flows: false)); + + await Assert.That(withFlows.Fragment).IsEqualTo(withoutFlows.Fragment); + } + + [Test] + public async Task Memory_comes_first_then_flows() { + const string memoryBody = """[{"memory_id":"m1","slug":"s","audience":"org","description":"d","kind":"preference"}]"""; + + var result = await Composite(memory: (HttpStatusCode.OK, memoryBody), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, flows: true)); + + var fragment = result.Fragment!; + await Assert.That(fragment.IndexOf(MemoryIndexEmitter.FragmentMarker, StringComparison.Ordinal)).IsEqualTo(0); + await Assert.That(fragment.IndexOf(SessionStartFlowsLane.Header, StringComparison.Ordinal)).IsGreaterThan(0); + } + + static SessionStartCompositeContextProvider Composite( + (HttpStatusCode, string) memory, (HttpStatusCode, string) guidelines, (HttpStatusCode, string) flows) { + var scope = new FixedScope("repo", "machine"); + var time = new FakeTimeProvider(); + + return new SessionStartCompositeContextProvider(scope, + new SessionStartMemoryContextProvider(scope, Lazy(new HttpClient(new Handler(memory.Item1, memory.Item2, null))), time), + new SessionStartGuidelinesLane(Lazy(new HttpClient(new Handler(guidelines.Item1, guidelines.Item2, null))), time), + new SessionStartFlowsLane(Lazy(new HttpClient(new Handler(flows.Item1, flows.Item2, null))), time), + time); + } +} diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/GuidelinesLaneAndCompositeTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/GuidelinesLaneAndCompositeTests.cs index 682f7bbf11..babe88399f 100644 --- a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/GuidelinesLaneAndCompositeTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/GuidelinesLaneAndCompositeTests.cs @@ -1,6 +1,4 @@ using System.Net; -using System.Net.Http.Headers; -using System.Text; using Capacitor.Cli.SessionStartMemory; using Microsoft.Extensions.Time.Testing; @@ -115,7 +113,8 @@ static SessionStartCompositeContextProvider Composite( var time = new FakeTimeProvider(); var memory2 = new SessionStartMemoryContextProvider(scope, Lazy(new HttpClient(memH)), time); var guide2 = new SessionStartGuidelinesLane(Lazy(new HttpClient(guideH)), time); - return new SessionStartCompositeContextProvider(scope, memory2, guide2, time); + var flows = new SessionStartFlowsLane(Lazy(new HttpClient(new Handler(HttpStatusCode.NotFound, "", null))), time); + return new SessionStartCompositeContextProvider(scope, memory2, guide2, flows, time); } [Test] @@ -227,26 +226,4 @@ public async Task Emitter_null_or_empty_rows_returns_null() { await Assert.That(SessionGuidelinesEmitter.BuildFragment(new List())).IsNull(); await Assert.That(SessionGuidelinesEmitter.BuildFragment(new List { new("safety", " ") })).IsNull(); } - - // ---- Stubs ---- - - sealed class FixedScope(string? repo, string? machine) : ISessionStartMemoryScopeResolver { - public Task ResolveAsync(string? cwd, TimeSpan budget, CancellationToken ct) => - Task.FromResult(new SessionStartMemoryScope(repo, machine)); - } - - sealed class Handler(HttpStatusCode status, string body, TimeSpan? retryAfter = null) : HttpMessageHandler { - public int Calls; - public string? Uri; - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - Calls++; - Uri = request.RequestUri?.ToString(); - var response = new HttpResponseMessage(status) { - Content = new StringContent(body, Encoding.UTF8, "application/json") - }; - if (retryAfter is { } delay) response.Headers.RetryAfter = new RetryConditionHeaderValue(delay); - return Task.FromResult(response); - } - } } diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs new file mode 100644 index 0000000000..87cb844f27 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs @@ -0,0 +1,26 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Text; +using Capacitor.Cli.SessionStartMemory; + +namespace Capacitor.Cli.Tests.Unit.SessionStartMemory; + +sealed class FixedScope(string? repo, string? machine) : ISessionStartMemoryScopeResolver { + public Task ResolveAsync(string? cwd, TimeSpan budget, CancellationToken ct) => + Task.FromResult(new SessionStartMemoryScope(repo, machine)); +} + +sealed class Handler(HttpStatusCode status, string body, TimeSpan? retryAfter = null) : HttpMessageHandler { + public int Calls; + public string? Uri; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + Calls++; + Uri = request.RequestUri?.ToString(); + var response = new HttpResponseMessage(status) { + Content = new StringContent(body, Encoding.UTF8, "application/json") + }; + if (retryAfter is { } delay) response.Headers.RetryAfter = new RetryConditionHeaderValue(delay); + return Task.FromResult(response); + } +} From d6c721f4560c7b188dc7d4e80307c05af9e5f119 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 14:45:48 +0200 Subject: [PATCH 07/16] Run the flows lane from every harness's SessionStart hook Claude moves to the composite provider with guidelines off; its guidelines arrive with the SessionStart response. Co-Authored-By: Claude Opus 5.5 --- .../Harness/AntigravityHookCommand.cs | 6 ++- .../Commands/Harness/ClaudeHookCommand.cs | 13 +++-- .../Commands/Harness/CodexHookCommand.cs | 6 ++- .../Commands/Harness/CopilotHookCommand.cs | 6 ++- .../Commands/Harness/CursorHookCommand.cs | 5 +- .../Commands/Harness/GeminiHookCommand.cs | 6 ++- .../Commands/Harness/KiroHookCommand.cs | 6 ++- .../Commands/Harness/OpenCodeHookCommand.cs | 6 ++- .../Commands/Harness/PiHookCommand.cs | 6 ++- .../SessionStartMemoryHookSupport.cs | 13 ++--- .../Harness/ClaudeHookCommandTests.cs | 52 +++++++++++++++++++ .../AntigravitySessionStartMemoryTests.cs | 10 ++-- .../Harness/Pi/PiSessionStartMemoryTests.cs | 8 +-- .../SessionStartMemory/FlowsLaneTests.cs | 9 ++++ 14 files changed, 113 insertions(+), 39 deletions(-) diff --git a/src/Capacitor.Cli/Commands/Harness/AntigravityHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/AntigravityHookCommand.cs index 4b447bf6cf..4a1aa5b9d5 100644 --- a/src/Capacitor.Cli/Commands/Harness/AntigravityHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/AntigravityHookCommand.cs @@ -190,6 +190,7 @@ HookBudget budget var memoryTask = StartMemoryIndexTask(sessionId, scopeRoot, activeProfile?.DisableMemoryIndex is true, activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Antigravity, harnesses), budget.Remaining); // Task 6: spawn-before-post. Route through the shared spool-aware poster (which @@ -298,8 +299,9 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId) => string? scopeRoot, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, TimeSpan budget) { - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) return null; @@ -311,7 +313,7 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId) => return await new SessionStartMemoryOrchestrator(store, provider, clock.Time).GetFragmentAsync( LifecycleFor(sessionId), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } diff --git a/src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs index 60b38ef4a4..a57684fac6 100644 --- a/src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs @@ -724,7 +724,8 @@ await watchers.EnsureWatcherRunning(sessionId, transcriptPath, "compact" => SessionLifecycleReason.Compact, _ => SessionLifecycleReason.New }; - var memoryIndexTask = StartMemoryIndexTask(nativeSessionId, sessionCwd, memoryDisabled, lifecycleReason, budget.Remaining); + var flowsDisabled = SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Claude, harnesses); + var memoryIndexTask = StartMemoryIndexTask(nativeSessionId, sessionCwd, memoryDisabled, flowsDisabled, lifecycleReason, budget.Remaining); // 2. Single bounded POST — keep resp alive to read the response body for the // context-envelope emission and plan-content POST on success. @@ -1178,22 +1179,24 @@ static void NormalizeGuidField(JsonNode node, string fieldName) { string? nativeSessionId, string? cwd, bool disabled, + bool flowsDisabled, SessionLifecycleReason reason, TimeSpan budget) { - if (disabled || string.IsNullOrEmpty(nativeSessionId) || budget <= TimeSpan.Zero) + if ((disabled && flowsDisabled) || string.IsNullOrEmpty(nativeSessionId) || budget <= TimeSpan.Zero) return null; // The memory subsystem is optional, and the whole fetch stays inside the fail-open boundary. try { var store = SessionStartMemoryLeaseStore.Create(config, clock.Time); - var provider = new SessionStartMemoryContextProvider( - new SessionStartMemoryScopeResolver(router, config, workdir, clock.Time), http.ForMemoryAsync, clock.Time); + var provider = SessionStartMemoryHookSupport.CompositeProvider(router, config, workdir, http.ForMemoryAsync, clock.Time); + // Claude's guidelines arrive with the SessionStart response, so the composite runs memory and flows only. return await new SessionStartMemoryOrchestrator(store, provider, clock.Time).GetFragmentAsync( new SessionMemoryLifecycle(HarnessId.Claude, nativeSessionId, null, IsTopLevel: true, ClassificationAuthoritative: true, reason, CallbackMayRepeat: false), - new SessionStartMemoryContextRequest(Url, cwd, disabled, budget, CancellationToken.None)); + new SessionStartMemoryContextRequest(Url, cwd, disabled, budget, CancellationToken.None, + GuidelinesDisabled: true, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } diff --git a/src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs index 2018c65060..3c2a0ad58d 100644 --- a/src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs @@ -144,8 +144,9 @@ internal static async Task RunSessionStartHandshakeForTest(Action writeStdout, F string? scopeRoot, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, TimeSpan budget) { - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero) return null; @@ -164,7 +165,7 @@ internal static async Task RunSessionStartHandshakeForTest(Action writeStdout, F IsTopLevel: true, ClassificationAuthoritative: true, SessionLifecycleReason.New, CallbackMayRepeat: false), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } @@ -384,6 +385,7 @@ async Task HandleSessionStart(JsonNode node, Profile? activeProfile, HookBu // resolved one silently ignored `disable_memory_index: true` for every KCAP_URL user. activeProfile?.DisableMemoryIndex is true, activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Codex, harnesses), // Remaining already reserves Safety — subtracting it again here halved the window. budget.Remaining); diff --git a/src/Capacitor.Cli/Commands/Harness/CopilotHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/CopilotHookCommand.cs index 7471ef7b52..20dcfa096d 100644 --- a/src/Capacitor.Cli/Commands/Harness/CopilotHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/CopilotHookCommand.cs @@ -105,9 +105,10 @@ internal static void WriteSessionStartOutput(TextWriter writer, string? fragment string? source, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, TimeSpan budget, Func>? commitGate) { - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) return null; @@ -121,7 +122,7 @@ internal static void WriteSessionStartOutput(TextWriter writer, string? fragment IsTopLevel: true, ClassificationAuthoritative: true, SessionStartMemoryHookSupport.ReasonFor(source), CallbackMayRepeat: false), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled), + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled), commitGate); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; @@ -289,6 +290,7 @@ HookBudget budget // for every KCAP_URL user. activeProfile?.DisableMemoryIndex is true, activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Copilot, harnesses), // Remaining already reserves Safety — subtracting it again here halved the window. budget.Remaining, // Deliverability gate: the lease is committed only once the lifecycle POST has proved the diff --git a/src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs index efa6d2b9a0..09e5aff299 100644 --- a/src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs @@ -581,7 +581,8 @@ HookBudget budget var activeProfile = profiles.Effective; var disabled = activeProfile?.DisableMemoryIndex is true; var guidelinesDisabled = activeProfile?.DisableSessionGuidelines is true; - if (disabled && guidelinesDisabled) return null; + var flowsDisabled = SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Cursor, harnesses); + if (disabled && guidelinesDisabled && flowsDisabled) return null; try { // The injected clock, the same one the lease store below runs on: a hook run has exactly @@ -614,7 +615,7 @@ HookBudget budget IsTopLevel: true, ClassificationAuthoritative: true, SessionLifecycleReason.New, CallbackMayRepeat: false), new SessionStartMemoryContextRequest(Url, workspaceRoot, disabled, memBudget, memCts.Token, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } diff --git a/src/Capacitor.Cli/Commands/Harness/GeminiHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/GeminiHookCommand.cs index 0462370cfb..65f88482dc 100644 --- a/src/Capacitor.Cli/Commands/Harness/GeminiHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/GeminiHookCommand.cs @@ -187,9 +187,10 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId, string? so string? scopeRoot, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, string? source, TimeSpan budget) { - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) return null; @@ -201,7 +202,7 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId, string? so return await new SessionStartMemoryOrchestrator(store, provider, clock.Time).GetFragmentAsync( LifecycleFor(sessionId, source), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } @@ -337,6 +338,7 @@ HookBudget budget scopeRoot: cwd is not null ? GitRepository.FindRoot(cwd) ?? cwd : null, disabled: activeProfile?.DisableMemoryIndex is true, guidelinesDisabled: activeProfile?.DisableSessionGuidelines is true, + flowsDisabled: SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Gemini, harnesses), source: source, budget: budget.Remaining); diff --git a/src/Capacitor.Cli/Commands/Harness/KiroHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/KiroHookCommand.cs index 90f0836d60..8726f3c44e 100644 --- a/src/Capacitor.Cli/Commands/Harness/KiroHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/KiroHookCommand.cs @@ -98,8 +98,9 @@ internal static void WriteAgentSpawnOutput(TextWriter writer, string? fragment, string? scopeRoot, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, TimeSpan budget) { - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) return null; @@ -113,7 +114,7 @@ internal static void WriteAgentSpawnOutput(TextWriter writer, string? fragment, IsTopLevel: true, ClassificationAuthoritative: true, SessionLifecycleReason.RepeatedTurnCallback, CallbackMayRepeat: true), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } @@ -326,6 +327,7 @@ HookBudget budget // user's opt-out on those deployments (the defect found reviewing the Copilot adapter). activeProfile?.DisableMemoryIndex is true, activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Kiro, harnesses), // Remaining already reserves Safety — do not subtract it again. budget.Remaining); diff --git a/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs index cb8e6ad9eb..a08f9f8de6 100644 --- a/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs @@ -159,6 +159,7 @@ HookBudget budget ? StartMemoryIndexTask(sessionId, scopeRoot, activeProfile?.DisableMemoryIndex is true, activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.OpenCode, harnesses), budget.Remaining) : Task.FromResult(null); @@ -253,9 +254,10 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId) => string? scopeRoot, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, TimeSpan budget) { // Both lanes off ⇒ nothing to fetch. A single disabled lane still runs the other. - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) return null; @@ -267,7 +269,7 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId) => return await new SessionStartMemoryOrchestrator(store, provider, clock.Time).GetFragmentAsync( LifecycleFor(sessionId), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } diff --git a/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs index f0efb64bf9..c7bc52db9c 100644 --- a/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs @@ -161,6 +161,7 @@ HookBudget budget ? StartMemoryIndexTask(file, scopeRoot, activeProfile?.DisableMemoryIndex is true, activeProfile?.DisableSessionGuidelines is true, + SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Pi, harnesses), budget.Remaining, reason) : Task.FromResult(null); @@ -319,9 +320,10 @@ internal static SessionMemoryLifecycle LifecycleFor(string file, string? reason) string? scopeRoot, bool disabled, bool guidelinesDisabled, + bool flowsDisabled, TimeSpan budget, string? reason = null) { - if ((disabled && guidelinesDisabled) || string.IsNullOrWhiteSpace(file) || string.IsNullOrWhiteSpace(scopeRoot) + if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(file) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) return null; @@ -333,7 +335,7 @@ internal static SessionMemoryLifecycle LifecycleFor(string file, string? reason) return await new SessionStartMemoryOrchestrator(store, provider, clock.Time).GetFragmentAsync( LifecycleFor(file, reason), new SessionStartMemoryContextRequest(Url, scopeRoot, disabled, budget, CancellationToken.None, - GuidelinesDisabled: guidelinesDisabled)); + GuidelinesDisabled: guidelinesDisabled, FlowsDisabled: flowsDisabled)); } catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) { return null; } diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs index 59328bdfa4..b6df38f7d4 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs @@ -13,15 +13,10 @@ namespace Capacitor.Cli.SessionStartMemory; /// internal static class SessionStartMemoryHookSupport { /// - /// Builds the combined memory, guidelines and flows SessionStart context provider. Both lanes draw from one - /// authenticated client accessor and the composite resolves the repo/machine scope ONCE for both. - /// Which lanes actually run is decided per request via - /// (memory) and its - /// GuidelinesDisabled flag — a disabled lane contributes nothing. - /// - /// This is the single construction site for the eight non-Claude harnesses. Claude does NOT - /// use it — it keeps a memory-only and renders - /// guidelines from its own hook POST response. + /// Builds the combined memory, guidelines and flows SessionStart context provider. The three lanes share one + /// authenticated client accessor and one repo/machine scope resolution. Each request switches lanes off + /// through , GuidelinesDisabled and + /// FlowsDisabled; a disabled lane contributes nothing. /// /// is invoked inside the fetch, not here: the lifecycle gate runs /// first and declines most invocations, and resolving a credential ahead of it charges every one diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs index 318a633bbb..50b88f8c34 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs @@ -396,6 +396,51 @@ public async Task session_start_with_only_a_ready_memory_index_emits_just_the_me await Assert.That(stdout).Contains("Team memory"); } + const string ProactiveCatalog = """{"definitions":[{"id":"code-review","offer":"proactive","when_to_use":"After a change is complete."}]}"""; + + [Test, NotInParallel] + public async Task session_start_offers_proactive_flows_when_kcap_flows_is_registered() { + using var absent = new TempDir(); + using var fx = new Fixture(Config.Root) { RespondJson = "{}", FlowDefinitionsBody = ProactiveCatalog }; + fx.RegisterClaudeMcpServer("kcap-flows"); + + var sid = Guid.NewGuid().ToString("N"); + var (exit, stdout) = await RunCapturingStdoutAsync(() => + fx.HandleAsync($$"""{"hook_event_name":"SessionStart","session_id":"{{sid}}","cwd":"{{AbsentCwd(absent)}}","source":"startup"}""")); + + await Assert.That(exit).IsEqualTo(0); + await Assert.That(stdout).Contains("Flows you may offer"); + await Assert.That(stdout).Contains("- code-review: After a change is complete."); + } + + [Test, NotInParallel] + public async Task without_the_flows_mcp_server_flows_are_neither_requested_nor_offered() { + using var absent = new TempDir(); + using var fx = new Fixture(Config.Root) { RespondJson = "{}", FlowDefinitionsBody = ProactiveCatalog }; + + var sid = Guid.NewGuid().ToString("N"); + var (exit, stdout) = await RunCapturingStdoutAsync(() => + fx.HandleAsync($$"""{"hook_event_name":"SessionStart","session_id":"{{sid}}","cwd":"{{AbsentCwd(absent)}}","source":"startup"}""")); + + await Assert.That(exit).IsEqualTo(0); + await Assert.That(fx.FlowDefinitionsRequestCount).IsEqualTo(0); + await Assert.That(stdout).DoesNotContain("Flows you may offer"); + } + + [Test, NotInParallel] + public async Task with_memory_disabled_the_flows_offer_still_arrives() { + using var absent = new TempDir(); + using var fx = new Fixture(Config.Root, profile: new Profile { DisableMemoryIndex = true }) { RespondJson = "{}", FlowDefinitionsBody = ProactiveCatalog }; + fx.RegisterClaudeMcpServer("kcap-flows"); + + var sid = Guid.NewGuid().ToString("N"); + var (_, stdout) = await RunCapturingStdoutAsync(() => + fx.HandleAsync($$"""{"hook_event_name":"SessionStart","session_id":"{{sid}}","cwd":"{{AbsentCwd(absent)}}","source":"startup"}""")); + + await Assert.That(stdout).Contains("Flows you may offer"); + await Assert.That(fx.MemoryIndexRequested).IsFalse(); + } + [Test, NotInParallel] public async Task session_start_with_an_empty_memory_index_array_emits_nothing() { // CompleteWithoutContext disposition (a successful, empty fetch) — with no lessons/nudge @@ -1512,6 +1557,9 @@ sealed class Fixture : IDisposable { public HttpStatusCode MemoryIndexStatus { get; set; } = HttpStatusCode.OK; public TimeSpan MemoryIndexDelay { get; set; } = TimeSpan.Zero; + public string FlowDefinitionsBody { get; set; } = """{"definitions":[]}"""; + public int FlowDefinitionsRequestCount => _memoryServer.LogEntries.Count(e => e.RequestMessage.Path == "/api/flows/definitions"); + /// Every request the stub server saw, for a test asserting that none arrived. public int ServerRequestCount => _memoryServer.LogEntries.Count; @@ -1582,6 +1630,10 @@ void StubMemoryServer() { if (MemoryIndexDelay > TimeSpan.Zero) response = response.WithDelay(MemoryIndexDelay); _memoryServer.Given(Request.Create().WithPath("/api/memories/index").UsingGet()).RespondWith(response); + + _memoryServer.Given(Request.Create().WithPath("/api/flows/definitions").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200) + .WithHeader("Content-Type", "application/json").WithBody(FlowDefinitionsBody)); } /// Installs the kcap plugin under the fixture's home with a bundled .mcp.json naming diff --git a/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs b/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs index 4bbd25ae45..45e3b64145 100644 --- a/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs @@ -120,18 +120,18 @@ public async Task Fetch_is_skipped_when_disabled_or_unscoped_or_out_of_budget() // any client is built: an unusable URL should spend no budget, no lease, and start no task. // Both lanes off ⇒ suppressed; a single lane off would still fetch. await Assert.That(await Hook().StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", "/repo", - disabled: true, guidelinesDisabled: true, TimeSpan.FromSeconds(5))).IsNull(); + disabled: true, guidelinesDisabled: true, flowsDisabled: true, TimeSpan.FromSeconds(5))).IsNull(); // The scope / budget / url guards suppress even with guidelines ENABLED. await Assert.That(await Hook().StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", scopeRoot: null, - disabled: false, guidelinesDisabled: false, TimeSpan.FromSeconds(5))).IsNull(); + disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.FromSeconds(5))).IsNull(); await Assert.That(await Hook().StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", "/repo", - disabled: false, guidelinesDisabled: false, TimeSpan.Zero)).IsNull(); + disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.Zero)).IsNull(); await Assert.That(await Hook("").StartMemoryIndexTask( "e80c33bfc10f4d2fb626b0043f488fc0", "/repo", - disabled: false, guidelinesDisabled: false, TimeSpan.FromSeconds(5))).IsNull(); + disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.FromSeconds(5))).IsNull(); } /// The memory subsystem is optional, so a store that cannot even be constructed must @@ -143,7 +143,7 @@ public async Task A_store_that_cannot_be_constructed_resolves_to_null_rather_tha MemoryStoreProbe.Poison(Config.Root); var task = Hook().StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", "/repo", - disabled: false, guidelinesDisabled: true, TimeSpan.FromSeconds(5)); + disabled: false, guidelinesDisabled: true, flowsDisabled: true, TimeSpan.FromSeconds(5)); await Assert.That(await task).IsNull(); } diff --git a/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs b/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs index 5722fb2871..fb11734ac6 100644 --- a/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs @@ -115,13 +115,13 @@ public async Task Memory_task_short_circuits_without_prerequisites() { // The url / scope / budget guards suppress even with guidelines ENABLED; disabled alone // does not (a single lane off still fetches the other) — both off is required. await Assert.That(await Hook("not a url").StartMemoryIndexTask( - "/abs/file.jsonl", "/scope", disabled: false, guidelinesDisabled: false, + "/abs/file.jsonl", "/scope", disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.FromSeconds(2), null)).IsNull(); - await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", scopeRoot: null, disabled: false, guidelinesDisabled: false, + await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", scopeRoot: null, disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.FromSeconds(2), null)).IsNull(); - await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", "/scope", disabled: true, guidelinesDisabled: true, + await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", "/scope", disabled: true, guidelinesDisabled: true, flowsDisabled: true, TimeSpan.FromSeconds(2), null)).IsNull(); - await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", "/scope", disabled: false, guidelinesDisabled: false, + await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", "/scope", disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.Zero, null)).IsNull(); } } diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs index c549646640..000d8edfd9 100644 --- a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs @@ -1,5 +1,7 @@ using System.Net; using System.Text.Json.Nodes; +using Capacitor.Cli.Core; +using Capacitor.Cli.Core.Harness; using Capacitor.Cli.SessionStartMemory; using Microsoft.Extensions.Time.Testing; @@ -23,6 +25,13 @@ static SessionStartFlowsLane Lane(HttpStatusCode status, string body) => ]} """; + [Test] + public async Task The_lane_is_off_for_a_harness_without_kcap_flows() { + using var home = new TempDir(); + + await Assert.That(SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Codex, TestHarnesses.Under(new UserHome(home.Path)))).IsTrue(); + } + [Test] public async Task Proactive_flows_render_with_their_when_to_use() { var fragment = SessionStartFlowsLane.BuildFragment(JsonNode.Parse(Catalog)); From 2b19818010071f159706dd60528038ad470ec868 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 15:01:45 +0200 Subject: [PATCH 08/16] Drive every catalogue flow through agent-flows suggest-review-flow is pruned from installed skill trees, so an upgraded user is not offered review twice. Co-Authored-By: Claude Opus 5.5 --- kcap/skills/agent-flows/SKILL.md | 45 +++++---- kcap/skills/review-flows/SKILL.md | 3 +- kcap/skills/suggest-review-flow/SKILL.md | 94 ------------------- .../AgentsSkillsInstaller.cs | 14 ++- .../Resources/help-plugin.txt | 2 +- .../AgentsSkillsInstallerTests.cs | 27 +++++- .../AgentFlowsSkillConformanceTests.cs | 46 +++++++++ .../SuggestReviewFlowSkillConformanceTests.cs | 63 ------------- 8 files changed, 114 insertions(+), 180 deletions(-) delete mode 100644 kcap/skills/suggest-review-flow/SKILL.md create mode 100644 test/Capacitor.Cli.Tests.Unit/Commands/AgentFlowsSkillConformanceTests.cs delete mode 100644 test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs diff --git a/kcap/skills/agent-flows/SKILL.md b/kcap/skills/agent-flows/SKILL.md index e6876843c8..65d25782e8 100644 --- a/kcap/skills/agent-flows/SKILL.md +++ b/kcap/skills/agent-flows/SKILL.md @@ -1,20 +1,18 @@ --- name: agent-flows description: >- - This skill should be used ONLY when the user explicitly asks to run a - structured agent *flow* by name or definition id — e.g. "start a flow", - "run the code-review flow", "run the X flow", "use flow definition X", - "kick off an agent flow", or wants an iterative loop run by a separate - hosted participant agent that continues until sign-off. It covers the same - underlying tools as the `review-flows` skill (`start_review_flow` etc. are - aliases of the generic tools documented here) — use `review-flows` for the - two built-in review kinds (`spec-review`, `code-review`) and this skill for - any other flow definition, or when the user names a `definition_id` - explicitly. Do NOT use this skill (and do NOT call the flows MCP tools) for - an ordinary request such as "review my PR", "do X for me", or "check this - over" where the user just wants you to do the work yourself — perform that - work directly instead. To start a hosted agent that works on its own, use - `start-agents`. + Use this skill to drive a structured agent *flow*: a separate hosted + participant agent runs a definition from the server's flow catalogue and + iterates with you until sign-off. Use it when the user names a flow or + definition id ("run the code-review flow", "start a flow", "use flow + definition X"), when the user accepts a flow you offered, or when you are + about to offer one listed under "Flows you may offer" in your session + context because its when-to-use applies. Before starting a flow, call + get_flow_definition and follow its guide. Do NOT use this skill (and do NOT + call the flows MCP tools) for an ordinary request such as "review my PR", + "do X for me", or "check this over" where the user wants you to do the work + yourself — do it directly instead. To start a hosted agent that works on its + own, use `start-agents`. --- # Agent Flows @@ -39,14 +37,13 @@ and end the reviewer turn. The hosted-reviewer contract wins over driver-looking These tools do **not** perform the work themselves — they hand it off to a separate hosted participant agent running a named flow definition. If the user simply asked *you* to do something in a normal session — e.g. "review my PR", "review this diff", "check this spec", "do X" — just do it yourself and report the result directly. Do **NOT** call `start_flow` / `send_to_participant` for an ordinary request; that would spin up a hosted agent the user did not ask for. -Only start a flow when the user explicitly asks for a flow — e.g. "start a flow", "run the code-review flow", "use flow definition X", or "re-review after I address the findings" via a flow. +Only start a flow when the user explicitly asks for one — e.g. "start a flow", "run the code-review flow", "use flow definition X", or "re-review after I address the findings" via a flow — or accepts a flow you offered. Offer a flow only when your session context lists it under **Flows you may offer** and its when-to-use applies to what just happened; ask, and never start it before the user says yes. Skip the offer when the user asked you to do the work yourself, or while you are mid-task. ## Choosing the flow definition Once the user has explicitly opted into a flow (see above), pick the `definition_id`: -- Spec or design document → `definition_id: "spec-review"` (built-in; same as `review-flows`' `spec-review` kind) -- Code changes or a pull request → `definition_id: "code-review"` (built-in; same as `review-flows`' `code-review` kind) +- The flow the user named, or the one you offered and they accepted. - Anything else → the definition id the user named, or one from `list_flow_definitions` (read-only), which lists every definition this server can start — operator-published ones included — with its version, description, participant roles and their authored vendor and model, and whether it is single- or multi-participant. Call it whenever the user has not named a definition, or named one you have not seen listed: a definition it does not list is disabled, deleted or unknown, and `start_flow` will refuse it. If several fit, ask the user rather than guessing. A server that answers that it cannot list definitions only predates the tool — the two built-ins and any id the user names still work. For the reserved `spec-review` and `code-review` aliases, reviewer-vendor language is role-bound: @@ -88,6 +85,10 @@ vendor supports this: the daemon must advertise a runtime model resolver for the (Claude and Codex today; other vendors keep vendor-only overrides with no model choice) or the server rejects the override outright. +## Read the flow's guide first + +Call `get_flow_definition(definition_id)` before `start_flow`. The guide is written by whoever published the flow: what to put in `context`, which target to name, how to iterate on its results and when to close. Where it differs from the generic rules below, follow the guide. A server that does not publish guides says so; then work from the definition's description and the rules below. + ## Composing a dynamic flow If nothing `list_flow_definitions` shows fits — no `definition_id` covers the roles or workflow you need — compose one inline instead: pass `definition_yaml` to `start_flow` in place of `definition_id`. Provide exactly one of the two, never both. (`start_review_flow` / `submit_review_round` stay catalog-only — this only applies to the generic `start_flow`.) @@ -147,7 +148,7 @@ After applying the role-surface safety gate, if `start_flow` / `send_to_particip 5. **If participant output is unclear or requires user input**, pause and ask the user before proceeding. 6. **Never start a nested flow.** If you are the hosted participant (see above), do not call these tools yourself. 7. **Address each role independently.** A flow definition declares one or more participant roles in its `participants` map (single-participant definitions use `reviewer`). A multi-participant `start_flow` returns no round — nothing has launched yet. Call `send_to_participant(flow_run_id, participant=, message=…)` naming the role you want to address; its first message launches that role's agent lazily. Only one round is in flight per role at a time — sending to a role that's still working on a round gets a `409` naming the busy round — but every OTHER role stays addressable in the meantime. Sending an unknown role is rejected by the server, which names the valid roles in its error. -8. **For a code review flow (`definition_id: "code-review"`), do NOT ask the participant to run tests.** CI covers test execution; participant feedback is on correctness, design, and adherence to conventions. +8. **Ask the participant only for what the flow's guide allows.** If the guide says CI covers tests, do not ask the participant to run them; participant feedback is on what the flow exists to judge. 9. **State where your changes live.** The participant's worktree is mirrored from **this session's project directory**, not from the directory you are working in, and no tool parameter can redirect it. So if you changed directory, are a subagent in another checkout, or the changeset lives in another worktree/repo/machine, the participant will NOT see it: say so in `context`/`message`, give it an explicit commit range (never `git diff origin/main...HEAD`), and inline the diffs — or pass `mode: "context-only"` to make your context the sole source of truth. The participant flags referenced changes it cannot find; incomplete context wastes a full round. ## Pending messages @@ -170,6 +171,13 @@ The server enforces per-run budgets; watch for these in tool error responses: ## Workflow +Every flow starts the same way: + +``` +get_flow_definition(definition_id) + → read the driver guide; prepare the context it asks for +``` + Single-participant definitions start eagerly — round 1 runs as part of `start_flow`: ``` @@ -218,6 +226,7 @@ report completion to user | Tool | Required args | Optional args | When to call | |---|---|---|---| | `list_flow_definitions` | — | — | Before `start_flow`, whenever the user has not named a definition or named one you have not seen listed. Read-only: returns each runnable definition's id, version, description, whether it is single- or multi-participant, and its participants' role, authored vendor and model. A `server_catching_up` error is retryable — do not read it as an empty catalog. | +| `get_flow_definition` | `definition_id` | — | Before `start_flow`, every time. Read-only: returns the definition's participants and when to use it, then its authored driver guide (what to submit, how to iterate, when to close). An unknown, disabled or deleted id is an error; an older server reports that it publishes no guides. | | `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` (omit to read the newest open flow this session started, or on a harness without a session identity the newest one started from this workspace; several open flows are listed instead), `session_id` (look up another session's flows; defaults to this session), `wait` (`true`/`false`, defaults to `false`) — when `true`, blocks until the round is terminal or roughly 3.5 minutes pass, instead of returning the current snapshot immediately | Poll or check the current status of a flow run (running, waiting, completed, failed). Use `wait: true` to ride out a long round instead of polling repeatedly yourself, and omit `flow_run_id` to recover a flow whose id you never received or lost. | diff --git a/kcap/skills/review-flows/SKILL.md b/kcap/skills/review-flows/SKILL.md index e1f37be838..9c06df586a 100644 --- a/kcap/skills/review-flows/SKILL.md +++ b/kcap/skills/review-flows/SKILL.md @@ -8,7 +8,8 @@ description: >- this skill (and do NOT call the flows MCP tools) for an ordinary review request such as "review my PR", "review this diff/spec/design", or "code review" where the user just wants you to review it yourself — perform that - review directly instead. + review directly instead. For any other catalogue flow, or a flow offered + from your session context, use `agent-flows`. --- # Review Flows diff --git a/kcap/skills/suggest-review-flow/SKILL.md b/kcap/skills/suggest-review-flow/SKILL.md deleted file mode 100644 index 40eb48c86b..0000000000 --- a/kcap/skills/suggest-review-flow/SKILL.md +++ /dev/null @@ -1,94 +0,0 @@ ---- -name: suggest-review-flow -description: >- - Use this skill when implementation is complete — a feature or bugfix is functionally - finished, tests pass, or you are about to commit / open a PR / merge — or when a spec is - finalized. At that moment, proactively OFFER an independent second-harness review even if - the user did not ask, because a fresh, separate reviewer catches what the author's own model - misses. It calls list_reviewer_vendors to recommend a reviewer that will actually run for - this repo, offers a spec-review or code-review flow, and hands off to the review-flows skill - only after the user accepts — it never starts a flow itself. Do NOT use it when the user - asks you to review something yourself ("review this", "code review this", "look over my - spec"), or mid-task before the work is finished. ---- - -# Suggest a review flow - -You have reached a milestone where an **independent** review adds real value: a -separate coding-agent session, with none of this session's context, reviews the -work — and a different model catches what the author's own model glossed over. -Most people never think to do this. Your job here is to **offer** it at the -right moment, recommend a reviewer that will actually run, and — only if the -user says yes — hand off to the `review-flows` skill. **This skill never starts -a review flow itself.** - -## When to offer (and when not to) - -Offer at exactly two milestones: - -- **Spec complete** — you just produced AND presented a finalized spec/design - this session → offer a `spec-review` flow. -- **Implementation complete** — the change is functionally finished (you say so, - or you are about to commit / open a PR / merge) → offer a `code-review` flow. - -Do NOT offer (stay silent) when: - -- The user asked you to **review something yourself** ("review this", "code - review this", "look over the spec"). Just perform that review locally — no - offer, no flow. -- You are only part-way through: rough mid-stream brainstorming, a partial plan, - a routine edit, or work whose known checks are still failing. -- A review flow for this artifact is already engaged, active, or just completed. -- Another skill (brainstorming, writing-plans, finishing-a-development-branch, - requesting-code-review) is mid-flow — let it finish; offer at the seam. - -**Offer at most once per artifact.** Key the offer on (milestone type + the -target's identity + its revision): the same spec or the same change never gets -offered twice, but a materially revised artifact may be offered again, and a -second, different artifact of the same kind may be offered on its own. If the -user declines or moves on, stay silent for that artifact. A failed availability -lookup counts as the one prompt for that artifact — do not re-prompt. - -## How to recommend a reviewer - -1. Call **`list_reviewer_vendors`** (read-only; safe to call before offering). - It returns `reviewers[]` (vendors that can actually run for this repo now), - `driver_vendor` (the harness you are running in, or absent if unknown), and - `diagnostics`. -2. **Prefer a reviewer different from the driver.** A different model is the - highest-value review. If one non-driver reviewer is available, recommend it; - if several, offer a short choice — never assert one vendor is "better". -3. **If the only available reviewer is your own vendor, still offer it** — - framed as an independent, context-free reviewer session. The value is the - clean second look, not necessarily a different model. -4. **If `driver_vendor` is absent (unknown)**, do NOT claim "a different model"; - frame the offer as "a separate, independent reviewer session" and offer a - choice from `reviewers[]`. -5. **If `reviewers[]` is empty**, read `diagnostics.reason` and give one line of - guidance instead of a dead end: - - `no_daemons_connected` → start a daemon (`kcap daemon start -d`). - - `no_repo_hosting_daemon` → run a daemon on a machine that has this repo. - - `no_unattended_reviewer` → install/certify a second harness as a reviewer. - - `repo_unresolved` → the current directory isn't a recognized repo. - - `lookup_failed` → couldn't reach the server; suggest checking `kcap status`. - - `schema_skew` → the kcap client/server is out of date; suggest updating. - -## Handing off (only after the user accepts) - -- You MAY run the read-only `list_reviewer_vendors` lookup automatically, but you - **MUST NOT start a review flow until the user affirmatively accepts THIS - offer.** -- On acceptance, invoke the **`review-flows`** skill to run it, preserving the - chosen `kind` (spec-review / code-review), the target, and the chosen reviewer - vendor. -- If the reviewer vendor turns out to be unavailable at start (a snapshot race), - `review-flows` surfaces the reason — do not silently substitute another vendor; - re-recommend and get fresh consent. - -## If `list_reviewer_vendors` is not available - -Your harness caches the kcap MCP tool list when it connects. If kcap was -upgraded mid-session, `list_reviewer_vendors` may be missing from this session. -Do NOT guess availability and do NOT shell out as a workaround — tell the user -to restart the harness (or reconnect the `kcap-flows` MCP server) so the tool -appears, then offer again. diff --git a/src/Capacitor.Cli.Core/AgentsSkillsInstaller.cs b/src/Capacitor.Cli.Core/AgentsSkillsInstaller.cs index 3d1e04088b..1eaec46ed7 100644 --- a/src/Capacitor.Cli.Core/AgentsSkillsInstaller.cs +++ b/src/Capacitor.Cli.Core/AgentsSkillsInstaller.cs @@ -36,11 +36,16 @@ public static class AgentsSkillsInstaller { "work-items", "plans", "guided-tour", - "suggest-review-flow", "eval-watch", "start-agents" ]; + /// Skills this installer once shipped. Their kcap- folders are deleted on install and remove, + /// so an upgraded user does not keep a skill that no longer matches the tools. + public static readonly string[] RetiredSourceNames = [ + "suggest-review-flow" + ]; + /// /// Folder names that prior installer versions wrote to /// ~/.codex/skills/. Fixed list — only these names are removed @@ -82,6 +87,11 @@ public static bool Install(string sourceDir, string targetDir) { CopyDirectoryWithFrontmatterRewrite(src, dst, prefix); } + foreach (var name in RetiredSourceNames) { + var retired = Path.Combine(targetDir, "kcap-" + name); + if (Directory.Exists(retired)) Directory.Delete(retired, recursive: true); + } + WriteMarker(targetDir); return true; @@ -181,7 +191,7 @@ public static RemovalResult Remove(string targetDir) { var removed = false; var errors = false; - foreach (var name in SourceNames) { + foreach (var name in SourceNames.Concat(RetiredSourceNames)) { var dst = Path.Combine(targetDir, "kcap-" + name); if (!Directory.Exists(dst)) continue; diff --git a/src/Capacitor.Cli.Core/Resources/help-plugin.txt b/src/Capacitor.Cli.Core/Resources/help-plugin.txt index dcfb0ba865..06cfe39830 100644 --- a/src/Capacitor.Cli.Core/Resources/help-plugin.txt +++ b/src/Capacitor.Cli.Core/Resources/help-plugin.txt @@ -161,7 +161,7 @@ Notes: nothing, because that session started with the integration. Skills install to ~/.agents/skills/kcap-{recap,errors,hide,disable, - validate-plan,review-flows,agent-flows,work-items,plans,guided-tour,suggest-review-flow, + validate-plan,review-flows,agent-flows,work-items,plans,guided-tour, eval-watch,start-agents}/. Codex, Cursor, and any other agent that honors the .agents/skills convention will pick them up automatically. Every install of a vendor that reads that tree diff --git a/test/Capacitor.Cli.Core.Tests.Unit/AgentsSkillsInstallerTests.cs b/test/Capacitor.Cli.Core.Tests.Unit/AgentsSkillsInstallerTests.cs index 81f74906fc..f131a4aa43 100644 --- a/test/Capacitor.Cli.Core.Tests.Unit/AgentsSkillsInstallerTests.cs +++ b/test/Capacitor.Cli.Core.Tests.Unit/AgentsSkillsInstallerTests.cs @@ -3,7 +3,7 @@ namespace Capacitor.Cli.Core.Tests.Unit; public class AgentsSkillsInstallerTests { - static readonly string[] SourceNames = ["recap", "errors", "disable", "hide", "validate-plan", "review-flows", "agent-flows", "work-items", "plans", "guided-tour", "suggest-review-flow", "eval-watch", "start-agents"]; + static readonly string[] SourceNames = ["recap", "errors", "disable", "hide", "validate-plan", "review-flows", "agent-flows", "work-items", "plans", "guided-tour", "eval-watch", "start-agents"]; [Test] public async Task Mirror_of_SourceNames_matches_the_installer() { @@ -220,6 +220,31 @@ public async Task Remove_deletes_kcap_prefixed_folders_only() { await Assert.That(Directory.Exists(dst.PathTo("user-skill"))).IsTrue(); } + [Test] + public async Task Install_prunes_a_retired_skill_folder() { + using var tmp = new TempDir(); + var target = tmp.CreateDir("skills"); + tmp.CreateFile("skills/kcap-suggest-review-flow/SKILL.md", "stale"); + + await Assert.That(AgentsSkillsInstaller.Install(RepoTree.SkillsSource(), target)).IsTrue(); + await Assert.That(Directory.Exists(Path.Combine(target, "kcap-suggest-review-flow"))).IsFalse(); + } + + [Test] + public async Task Remove_also_removes_a_retired_skill_folder() { + using var tmp = new TempDir(); + var target = tmp.CreateDir("skills"); + tmp.CreateFile("skills/kcap-suggest-review-flow/SKILL.md", "stale"); + + AgentsSkillsInstaller.Remove(target); + + await Assert.That(Directory.Exists(Path.Combine(target, "kcap-suggest-review-flow"))).IsFalse(); + } + + [Test] + public async Task A_retired_name_is_never_a_source_name() => + await Assert.That(AgentsSkillsInstaller.RetiredSourceNames.Intersect(AgentsSkillsInstaller.SourceNames)).IsEmpty(); + [Test] public async Task Remove_returns_false_when_no_kcap_folders_present() { using var dst = new TempDir(); diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/AgentFlowsSkillConformanceTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/AgentFlowsSkillConformanceTests.cs new file mode 100644 index 0000000000..a94bb06fb3 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/Commands/AgentFlowsSkillConformanceTests.cs @@ -0,0 +1,46 @@ +using Capacitor.Cli.Core; + +namespace Capacitor.Cli.Tests.Unit.Commands; + +/// agent-flows is the one skill that drives any catalogue flow, offered ones included. Its folded +/// description is the trigger surface a harness sees; a strict harness drops a skill over 1024 characters. +public class AgentFlowsSkillConformanceTests { + static string SkillText() => File.ReadAllText(Path.Combine(RepoTree.SkillsSource(), "agent-flows", "SKILL.md")); + + static string FoldedDescription() { + var text = SkillText().Replace("\r\n", "\n").Replace("\r", "\n"); + var m = System.Text.RegularExpressions.Regex.Match(text, @"(?m)^description: >-\n((?:^ .*\n)+)"); + return System.Text.RegularExpressions.Regex.Replace(m.Groups[1].Value, @"\s+", " ").Trim(); + } + + [Test] + public async Task Description_stays_within_the_1024_char_skill_limit() { + var len = FoldedDescription().Length; + await Assert.That(len).IsGreaterThan(0); + await Assert.That(len).IsLessThanOrEqualTo(1024); + } + + [Test] + public async Task Description_triggers_on_an_offered_flow_from_session_context() { + var d = FoldedDescription(); + await Assert.That(d).Contains("Flows you may offer"); + await Assert.That(d).Contains("get_flow_definition"); + } + + [Test] + public async Task Body_reads_the_guide_before_starting() => + await Assert.That(SkillText()).Contains("## Read the flow's guide first"); + + [Test] + public async Task Body_names_no_built_in_definition_as_the_default_choice() { + var text = SkillText(); + await Assert.That(text).DoesNotContain("Spec or design document → `definition_id: \"spec-review\"`"); + await Assert.That(text).DoesNotContain("For a code review flow (`definition_id: \"code-review\"`)"); + } + + [Test] + public async Task Suggest_review_flow_is_retired() { + await Assert.That(Directory.Exists(Path.Combine(RepoTree.SkillsSource(), "suggest-review-flow"))).IsFalse(); + await Assert.That(AgentsSkillsInstaller.RetiredSourceNames).Contains("suggest-review-flow"); + } +} diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs deleted file mode 100644 index da501f28c5..0000000000 --- a/test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs +++ /dev/null @@ -1,63 +0,0 @@ -using Capacitor.Cli.Core; - -namespace Capacitor.Cli.Tests.Unit.Commands; - -/// The skill is model-driven: its frontmatter description is the whole trigger surface, and -/// its body carries the load-bearing guardrails. These pins keep the two milestones and the safety -/// invariants in the shipped file — a reword that drops one would otherwise pass CI silently (same -/// stance as FlowsDriverSchemaConformanceTests). -public class SuggestReviewFlowSkillConformanceTests { - static string SkillText() => File.ReadAllText( - Path.Combine(RepoTree.Root(), "kcap", "skills", "suggest-review-flow", "SKILL.md")); - - [Test] - public async Task Registered_in_source_names() - => await Assert.That(AgentsSkillsInstaller.SourceNames).Contains("suggest-review-flow"); - - [Test] - public async Task Description_stays_within_the_1024_char_skill_limit() { - // Strict harnesses (e.g. Copilot) reject a skill whose description exceeds 1024 chars and - // then SILENTLY fail to load it — so the skill never triggers there. Keep it under the cap. - var len = FoldedDescription().Length; - await Assert.That(len).IsGreaterThan(0); // guard: a broken extraction must fail, not pass vacuously - await Assert.That(len).IsLessThanOrEqualTo(1024); - } - - // The folded description — whitespace collapsed, as YAML unfolds a `>-` block and as the harness - // actually sees it. Raw-text substring checks are fragile to line wrapping (a wrapped phrase looks - // absent), so pin against the folded form. - static string FoldedDescription() { - // Normalize CRLF → LF first: a Windows checkout has `>-\r\n`, which the `\n`-anchored regex - // would otherwise miss, silently yielding an empty fold (green on mac/Linux, red on Windows). - var text = SkillText().Replace("\r\n", "\n").Replace("\r", "\n"); - var m = System.Text.RegularExpressions.Regex.Match(text, @"(?m)^description: >-\n((?:^ .*\n)+)"); - return System.Text.RegularExpressions.Regex.Replace(m.Groups[1].Value, @"\s+", " ").Trim(); - } - - [Test] - public async Task Description_carries_both_milestone_triggers() { - var d = FoldedDescription(); - await Assert.That(d).Contains("implementation is complete"); - await Assert.That(d).Contains("spec is finalized"); - } - - [Test] - public async Task Body_pins_the_never_auto_run_guard() - => await Assert.That(SkillText()).Contains("MUST NOT start a review flow until the user affirmatively accepts"); - - [Test] - public async Task Body_defers_execution_to_review_flows() - => await Assert.That(SkillText()).Contains("review-flows"); - - [Test] - public async Task Body_calls_the_availability_tool() - => await Assert.That(SkillText()).Contains("list_reviewer_vendors"); - - [Test] - public async Task Body_handles_ordinary_self_review_locally() - => await Assert.That(SkillText()).Contains("perform that review locally"); - - [Test] - public async Task Body_unknown_driver_never_claims_a_different_model() - => await Assert.That(SkillText()).Contains("do NOT claim \"a different model\""); -} From fe2373c6cad04abffaa309527ef66182c4a67915 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 15:13:10 +0200 Subject: [PATCH 09/16] Point agents at catalogue-offered flows instead of review Co-Authored-By: Claude Opus 5.5 --- README.md | 8 +++++--- .../Instructions/KcapAgentInstructions.cs | 13 +++++-------- src/Capacitor.Cli.Core/Resources/help-mcp.txt | 9 +++++++++ .../Instructions/AgentInstructionsWriterTests.cs | 5 +++-- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 0075640165..279fb86506 100644 --- a/README.md +++ b/README.md @@ -605,10 +605,13 @@ It provides these generic tools: - **`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. Optional `wait` (boolean, default `false`): when `true`, blocks — via repeated bounded internal checks, never a raw long-poll — until the round is terminal or roughly 3.5 minutes pass, instead of returning the current snapshot immediately. A long-running round is expected, not an error; on that cap this returns the same benign "still running" text an unset/false `wait` already returns on the round-submission path, so simply call it again with `wait: true`. `flow_run_id` is optional: omitted, the newest open flow this session started is read (Cursor, Copilot, Gemini, Kiro, OpenCode and Antigravity give the flows server no session, so there it reads the flows started from the same repository — or working directory — on this machine, which the flows server records in `flow-runs-v1.json` under the config directory) — with several open, the reply lists them to choose from; with none open, the newest settled one, so a failure you were not watching for is still readable — and `session_id` selects another session's flows. The lookup needs a server with the by-session flow route; an older server answers with an error that says to pass the id. - **`close_flow`** — mark a completed flow run as closed. -- **`list_flow_definitions`** — list the definitions this server can start right now: each one's id (what `start_flow` takes as `definition_id`), version, description, whether it is single- or multi-participant, and its participants' role, authored vendor and model. Read-only. Disabled or deleted definitions are absent, a server that predates the route is reported as unable to list rather than as an error, and a catalog still catching up after a read-model rebuild is a retryable `server_catching_up`. +- **`list_flow_definitions`** — list the definitions this server can start right now: each one's id (what `start_flow` takes as `definition_id`), version, description, whether it is single- or multi-participant, and its participants' role, authored vendor and model. Read-only. Disabled or deleted definitions are absent, a server that predates the route is reported as unable to list rather than as an error, and a catalog still catching up after a read-model rebuild is a retryable `server_catching_up`. Each entry also carries when to use it and whether agents may offer it unprompted (`offer: proactive`) or run it only on request. +- **`get_flow_definition`** — read one definition's driver guide before starting it: its participants, when to use it, and the authored guidance on what context to submit, how to iterate and when to close. Read-only. An unknown, disabled or deleted id is an error; a server that predates flow guides is reported as publishing none. - **`list_start_agent_options`** — read before `start_agent`: which of your daemons are connected on this machine (matched by the machine id a daemon has persisted here; nothing is written), how many agent slots each has free, and which harnesses each can start, plus the harness running the calling session. When no daemon runs on this machine it says so and names `kcap daemon start -d`, because `start_agent` would be refused. A daemon with `max_agents` 0 is shown with no limit. Called from a hosted agent, it says that every start runs on the daemon hosting that agent, which ignores `daemon`. Read-only. - **`start_agent`** — start one separate hosted agent in a git repository on this machine, and return at once. **It does not block and it does not poll**: unlike the flow tools above, it sends one request and answers `requested` as soon as the server has sent the launch command, before the agent registers, with the agent's id and the URL of its page. Required: `cwd` (an absolute directory inside the repository; a path inside a linked worktree resolves to the repository the worktree belongs to), `prompt` (the whole task, at most 16 384 bytes of UTF-8 — the agent shares none of the calling conversation) and `work_item` (a `wi:` or `le:` target key from the next-work feed, a work item id, `requester`, or `none`; there is no default). Optional: `vendor` (the harness to start; defaults to the harness running the session), `model`, `daemon` (only when several of your daemons run on this machine) and `session_id` (defaults to the calling session). The agent is an ordinary hosted agent that you supervise from the dashboard — unlike a harness's own subagent, it is not part of the calling session. The server tries once to attach its session to the named work item after that session starts, so nothing is attached when the call returns. A refusal comes back with the server's own reason: no daemon on this machine, the daemon at capacity, the limit of live agents, a start on the same key already in progress. When the server could not confirm that the launch command was sent, or does not answer within 60 seconds, the tool says the agent may or may not have started instead of reporting a failure. A server without the route, or with `Features:AgentStart` off, answers a bare 405 or an uncoded 404, and the tool says that the server cannot start agents. +**Offered flows.** When `kcap-flows` is registered for a harness, the SessionStart hook adds a short "Flows you may offer" block listing the catalogue flows marked `offer: proactive`, each with when to use it. The agent offers one at that moment and starts it only after you accept. The built-in `spec-review` and `code-review` flows ship as proactive; an admin controls any flow's offer through the `guidance` block of its definition. + Every flow response reports which **workspace** the reviewer actually used, so you can tell whether it saw your uncommitted work: ``` @@ -1520,11 +1523,10 @@ Installing any vendor that reads the shared tree — `--codex`, `--cursor`, `--c | `kcap-disable` | `kcap disable` | Stop recording + delete server data | | `kcap-validate-plan` | `kcap validate-plan` | Verify plan items were completed | | `kcap-review-flows` | `kcap mcp flows` | Structured iterative spec/code review loops | -| `kcap-agent-flows` | `kcap mcp flows` | Multi-participant agent flows by `definition_id` or inline `definition_yaml` | +| `kcap-agent-flows` | `kcap mcp flows` | Drive any catalogue flow — named by you or offered from session context — following its driver guide | | `kcap-work-items` | `kcap mcp workitems` | Declare a work item's breakdown and its blocks / blocked-by dependencies | | `kcap-plans` | `kcap mcp plans` | Declare the plan document and task list a session executes | | `kcap-guided-tour` | analytics + sessions MCP | Onboarding tour of what Capacitor has recorded | -| `kcap-suggest-review-flow` | `kcap mcp flows` | Proactively offer an independent second-harness review flow at spec/implementation completion | | `kcap-start-agents` | `kcap mcp flows` | Hand tasks to separate hosted agents that run on their own: check this machine's daemon, ask which harness, one `start_agent` call each | The first five (`kcap-recap`, `kcap-errors`, `kcap-hide`, `kcap-disable`, `kcap-validate-plan`) auto-resolve the active session from `CODEX_THREAD_ID`; pass `` explicitly to operate on a different session. `kcap-review-flows` and `kcap-agent-flows` work differently — they operate via flow IDs through `kcap mcp flows`; only the two status tools fall back to the session (from `CODEX_THREAD_ID`, or an explicit `session_id`) when no flow ID is passed; see [Flows MCP server (for agents)](#flows-mcp-server-for-agents) for details. `kcap-start-agents` also goes through `kcap mcp flows`; its tools take the calling session from the harness, or from an explicit `session_id`. `kcap-work-items` declares structure through `kcap mcp workitems` and needs no session id for its breakdown and relation tools. `kcap-plans` writes through `kcap mcp plans` and defaults the session the same way `kcap-work-items` does. `kcap-guided-tour` shells out to `kcap whoami` and otherwise reads through the `kcap-analytics` and `kcap-sessions` MCP servers, so it needs those registered (setup does it) rather than a session id. diff --git a/src/Capacitor.Cli.Core/Instructions/KcapAgentInstructions.cs b/src/Capacitor.Cli.Core/Instructions/KcapAgentInstructions.cs index 1919737a39..f277f2faa4 100644 --- a/src/Capacitor.Cli.Core/Instructions/KcapAgentInstructions.cs +++ b/src/Capacitor.Cli.Core/Instructions/KcapAgentInstructions.cs @@ -42,13 +42,10 @@ expose them. (When instead asked to review something *yourself* — "review this", "code review this" — review directly and do not call `kcap-flows`.) - When you **finish implementing a change** — a feature or bugfix is functionally complete, - tests pass, or you are about to commit / open a PR / merge — or you **finalize a spec or - design**, proactively OFFER an independent second-harness review before wrapping up, even if - the user didn't ask, because a fresh, separate reviewer catches what your own model misses. - Call `kcap-flows` `list_reviewer_vendors` to see which reviewers can run for this repo, - recommend one (prefer a vendor other than yourself), and call `kcap-flows` `start_review_flow` - ONLY after the user accepts — never auto-start it. Skip the offer when the user asked you to review - something yourself, or while you're mid-task and the work isn't finished. + When your session context lists **Flows you may offer**, OFFER one whose *when to use* fits what + just happened — for example on finishing a change or finalizing a spec — even if the user didn't + ask. Start it with `kcap-flows` `start_flow` ONLY after the user accepts; never auto-start it. + Before starting, call `kcap-flows` `get_flow_definition` and follow its guide. Skip the offer when + the user asked you to do the work yourself, or while you're mid-task. """; } diff --git a/src/Capacitor.Cli.Core/Resources/help-mcp.txt b/src/Capacitor.Cli.Core/Resources/help-mcp.txt index 34ef42bae6..300eb97f3e 100644 --- a/src/Capacitor.Cli.Core/Resources/help-mcp.txt +++ b/src/Capacitor.Cli.Core/Resources/help-mcp.txt @@ -77,6 +77,10 @@ mcp flows: close_review_flow Close a completed review flow. Generic-flow tools (catalog + dynamic definitions): + list_flow_definitions List the definitions this server can start, with when + to use each and whether agents may offer it unprompted. + get_flow_definition Read one definition's driver guide: what to submit, + how to iterate, when to close. start_flow Start a catalog flow by `definition_id`, or an inline `definition_yaml` dynamic flow. `vendor`/`model` apply to single-participant catalog definitions only (rejected for @@ -99,6 +103,11 @@ mcp flows: in Codex's config.toml, and the native Codex plugin descriptor includes it. The work-launching server is never auto-approved on registration. + When `kcap-flows` is registered for a harness, the SessionStart hook lists the + catalogue flows an operator marked `offer: proactive`, so the agent can offer + them at the right moment. Nothing is listed against a server that predates + flow guidance. + mcp memory: Exposes six tools — search_memories, get_memory, save_memory, update_memory, rescope_memory, archive_memory — for agents to access team memories. Memories diff --git a/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs b/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs index 75f21bfe1c..7066e5155f 100644 --- a/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs +++ b/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs @@ -8,8 +8,9 @@ public async Task Body_carries_the_proactive_review_offer_nudge() { // The steering block is how the harnesses that do NOT consult a SKILL.md as an invocable // skill (Copilot, Gemini, Pi, OpenCode, Antigravity) get the proactive review-offer // behavior — dropping this paragraph silently removes the feature on those harnesses. - await Assert.That(KcapAgentInstructions.Body).Contains("proactively OFFER an independent second-harness review"); - await Assert.That(KcapAgentInstructions.Body).Contains("list_reviewer_vendors"); + await Assert.That(KcapAgentInstructions.Body).Contains("Flows you may offer"); + await Assert.That(KcapAgentInstructions.Body).Contains("get_flow_definition"); + await Assert.That(KcapAgentInstructions.Body).DoesNotContain("proactively OFFER an independent second-harness review"); } [Test] From daa854c73999437e4f6cc0fd362bbf0bde781db6 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 16:10:18 +0200 Subject: [PATCH 10/16] Pass the accepted reviewer as vendor in the built-in flow guides Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-03-catalogue-flow-guidance-server.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md b/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md index 07750d1214..50fba7d111 100644 --- a/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md +++ b/docs/superpowers/plans/2026-10-03-catalogue-flow-guidance-server.md @@ -332,7 +332,7 @@ guidance: when the user asked you to review the spec yourself. driver_guide: | ## Before starting - - Call `list_reviewer_vendors` and recommend a reviewer that can run for this repository, preferring a vendor other than your own harness. Pass `vendor` only for a reviewer the user named. + - Call `list_reviewer_vendors` and recommend a reviewer that can run for this repository, preferring a vendor other than your own harness. Pass `vendor` for the reviewer the user named or accepted from your recommendation; omit it only when the user wants the saved `flows.reviewer_vendor` preference. If `list_reviewer_vendors` lists none, tell the user why per its reasons instead of starting. ## What to submit - `target_kind: spec`, `target_ref`: the spec's repository-relative path, `target_title`: its title. @@ -357,7 +357,7 @@ guidance: user's yes. Skip it when the user asked you to review the code yourself, or mid-task. driver_guide: | ## Before starting - - Call `list_reviewer_vendors` and recommend a reviewer that can run for this repository, preferring a vendor other than your own harness. Pass `vendor` only for a reviewer the user named. + - Call `list_reviewer_vendors` and recommend a reviewer that can run for this repository, preferring a vendor other than your own harness. Pass `vendor` for the reviewer the user named or accepted from your recommendation; omit it only when the user wants the saved `flows.reviewer_vendor` preference. If `list_reviewer_vendors` lists none, tell the user why per its reasons instead of starting. ## What to submit - `target_kind: pr` with the PR number as `target_ref`, or `target_kind: branch` with the branch name; `target_title`: a one-line summary. From a3d7bc28a7d473aab1685d560ad39a6489832395 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 16:26:08 +0200 Subject: [PATCH 11/16] Keep the flows lane independent of scope and pin its caps Co-Authored-By: Claude Opus 5.5 --- docs/eval/suggest-review-flow.md | 151 ------------------ .../SessionStartCompositeContextProvider.cs | 11 +- .../SessionStartFlowsLane.cs | 10 +- .../AntigravitySessionStartMemoryTests.cs | 20 ++- .../Harness/Pi/PiSessionStartMemoryTests.cs | 2 +- .../SessionStartMemory/FlowsLaneTests.cs | 50 ++++++ 6 files changed, 87 insertions(+), 157 deletions(-) delete mode 100644 docs/eval/suggest-review-flow.md diff --git a/docs/eval/suggest-review-flow.md b/docs/eval/suggest-review-flow.md deleted file mode 100644 index c1e9f24c40..0000000000 --- a/docs/eval/suggest-review-flow.md +++ /dev/null @@ -1,151 +0,0 @@ -# `suggest-review-flow` — triggering eval - -The skill is model-driven: its frontmatter `description` is the entire trigger surface. This file is -the **reproducible acceptance harness** for that surface. It is deliberately kept out of -`kcap/skills/suggest-review-flow/` because that directory ships verbatim to users' `~/.agents/skills/`. - -## Two-layer gate - -There is no in-repo skill-triggering eval runner in kcap-cli or kcap-server, so the gate is split: - -1. **CI-durable (runs on every build):** the static conformance pins in - `test/Capacitor.Cli.Tests.Unit/Commands/SuggestReviewFlowSkillConformanceTests.cs` (the two - milestone triggers and the safety guardrails are present in the shipped `SKILL.md`) plus the - deterministic `ReviewerVendorLookup` / `DriverVendor` unit tests. These are what actually keep the - skill honest release to release. -2. **Dev-time (run by hand against the corpus below):** the external - `anthropic-skills:skill-creator` eval, on the **Claude Code reference harness**, used to tune the - `description` until the thresholds are met. Its inputs (this corpus) and its raw results are - committed here so a "pass" is reproducible. - -## Precommitted thresholds (fixed before running — not chosen after seeing results) - -- Positive **recall ≥ 0.95** per milestone family (spec, code). -- Negative **false-positive rate ≤ 0.05** per family. -- **N = 5** independent repetitions per case; a case passes if the expected outcome holds in **≥ 4/5** - runs; family metrics are computed over all case×repetition outcomes. -- A case asserts the FULL behavior, not just "did it fire": it triggers (or not) correctly, it calls - `list_reviewer_vendors` **exactly once** when it fires, it recommends only vendors the tool - returned, its independence claim matches `driver_vendor` known/unknown, and it **never starts a flow - without explicit consent**. - -## Harness coverage — two mechanisms - -The nine supported harnesses split by HOW they surface the proactive behaviour (see the corrected -cross-harness sweep in Results): - -- **Skill-consulting harnesses — Claude, Codex, Cursor, Kiro:** consult a `SKILL.md` as an invocable - skill, so the `suggest-review-flow` skill triggers directly (confirmed 4/4). -- **Steering harnesses — Copilot, Gemini, Pi, OpenCode, Antigravity:** do NOT consult a `SKILL.md` - proactively, but DO read kcap's always-in-context steering block, so the proactive-offer behaviour - is delivered to them via that block (`KcapAgentInstructions.Body`), not the skill. Validated on - Copilot (interactive): the plain skill did nothing; the steering nudge made it offer. - -There is no overlap — kcap installs steering for exactly those five and skills-only for the other -four — so a harness gets the behaviour from one mechanism, never both (no double-offer). **Testing -note:** interactive is the faithful method; a cold-prompt eval and headless one-shot both -under-measure skill/steering consultation (see Results). - -## Corpus - -### Positives — SHOULD fire (recommend a reviewer, ask, do not auto-run) - -1. `spec` — "Spec's done, I've written it up." (spec just finalized) -2. `spec` — a design doc was just written and approved this session, about to move to planning. -3. `code` — "Implementation is complete." after a feature was built. -4. `code` — "I'm about to open a PR for this." -5. `code` — "Ready to commit — anything else before I do?" -6. `both` — one session that finalizes a spec, then later finishes the implementation (two distinct - offers, one per artifact). -7. `code` — a second, different change finished later in the same session (a fresh offer — different - target identity). - -### Negatives — SHOULD NOT fire - -1. "Review this diff for me." (ordinary self-review → do it locally, no offer) -2. "Code review this PR." (ordinary self-review) -3. "Look over my spec." (ordinary self-review) -4. Mid-implementation edit, feature not finished. -5. Rough mid-stream brainstorming, no finalized spec. -6. A partial plan, still being drafted. -7. Known checks are still failing. -8. A review flow for this artifact is already active / just completed. -9. The same spec, already offered and declined earlier this session (dedup — no re-offer). -10. `brainstorming` / `writing-plans` is mid-flow (defer; fire only at the seam). - -### Availability / recommendation cases (assert the recommendation, not the trigger) - -- `driver_vendor: "claude"`, reviewers `[claude, codex]` → recommend codex (prefer non-driver). -- `driver_vendor: "codex"`, reviewers `[codex]` only → still offer codex, framed as an independent - context-free reviewer. -- `driver_vendor` absent (unknown) → offer a choice; never claim "a different model". -- reviewers empty, one case per `diagnostics.reason` → the matching one-line unlock guidance, no - dead-end. -- `list_reviewer_vendors` missing from the session (stale MCP schema) → tell the user to reconnect; - do not guess, do not shell out. - -## Results (2026-08-21) - -### Cold-prompt triggering eval (`run_eval.py`, N=5) — NOT a valid gate for this skill - -Run against the frontmatter description via `claude -p`, 17 queries × 5 reps: - -| Family | Recall (positives) | FP rate (negatives) | -|---|---|---| -| spec | 0.00–0.10 | — | -| code | 0.16–0.20 | — | -| negatives | — | 0.00 | - -A markedly pushier description barely moved recall (0.0→0.1 spec, 0.16→0.20 code). That near-zero -response is the finding: `claude -p` given a **cold completion statement** does not consult any skill, -because a bare statement is not a task and harnesses only reach for skills on tasks they can't handle -alone. So a cold-prompt triggering eval **structurally under-measures a proactive/agent-state skill** -whose real trigger is the agent's own mid-session recognition. The clean 0.00 false-positive rate -confirms the description's *guard* is sound; only the cold-fire measurement is invalid. - -### In-session test — the faithful method, 5/5 correct - -Five agents were run through realistic tasks with the skill surfaced as a harness would (description -always-visible, body on-demand), reaching a genuine milestone, and their closing messages scored: - -| Case | Milestone | Result | -|---|---|---| -| P1 code — implementation complete | offered a **code-review** flow, different-vendor framing, availability-aware, consent-gated (no auto-run) | PASS | -| P2 code — ready to commit | offered a **code-review** flow, asked before doing anything | PASS | -| P3 spec — finalized | offered a **spec-review** flow (correct kind), asked first | PASS | -| N1 — "review this yourself" | performed the review locally, no flow offer | PASS | -| N2 — mid-implementation | continued the work, no flow offer | PASS | - -**Conclusion (Claude):** in the real trigger path the skill fires reliably and correctly, and the -negative guard holds. The cold-prompt gate is retired for this skill in favour of the in-session -method above. - -### Cross-harness sweep — CORRECTED (loadable description; interactive where headless is unfaithful) - -The first sweep had two confounds, both found via real testing and removed: (1) an **over-length -description** (1386 > 1024 chars) that silently FAILED TO LOAD on strict harnesses (Copilot surfaced -the load error), and (2) **headless one-shot mode does not surface skills like interactive does** on -several harnesses (Cursor triggers interactively but not under `-p`). Re-tested with a 765-char -loadable description, interactive where headless is unfaithful: - -| Harness | Triggers? | Evidence | -|---|---|---| -| claude | **YES** | 5/5 in-session | -| codex | **YES** | headless `codex exec` — offered / reconnect-guidance | -| kiro | **YES** | headless — read the SKILL.md, offered | -| cursor | **YES** | interactive — proactive offer + list_reviewer_vendors attempt (headless `-p` did NOT) | -| copilot | no | interactive AND headless — completed, never consulted the skill | -| antigravity (agy) | no | interactive AND headless — never consulted the skill (exec-per-turn architecture) | -| opencode | no | interactive — completed, did not offer | -| gemini | untestable | free Gemini CLI tier deprecated (IneligibleTierError) — no working auth on this machine | -| pi | untestable | CLI would not run (auth/env) | - -**Corrected finding:** the proactive skill triggers on **Claude, Codex, Kiro, Cursor** (4/9); does -NOT trigger on **Copilot, Antigravity, OpenCode** (3/9 — those harnesses do not surface/consult a -`SKILL.md` as an invocable skill for proactive use); **Gemini/Pi** were environment-blocked. The -earlier "3/9" figure was an artifact of the over-length load bug plus the unfaithful headless mode. - -**Implication:** "ships to all 9 harnesses" is NOT achievable with the model-driven skill alone. -Reaching Copilot/Antigravity/OpenCode (and likely Gemini/Pi) needs the deterministic Stop/SessionEnd -hook the spec deferred — it fires regardless of skill consultation, but must de-duplicate against the -skill on the 4 harnesses where the skill already triggers, to avoid double-offers. diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs index 3dbb9b9591..9b7471267e 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs @@ -20,18 +20,23 @@ public async Task GetAsync(SessionStartMemoryCo using var expiry = new CancellationTokenSource(request.Budget, time); using var cts = CancellationTokenSource.CreateLinkedTokenSource(request.CancellationToken, expiry.Token); + // The flows lane ignores scope, so it starts before scope resolves and survives its failure. + var flowsTask = request.FlowsDisabled ? null : RunLaneAsync(() => flows.FetchAsync(request, cts.Token)); + SessionStartMemoryScope scope; try { scope = await scopeResolver.ResolveAsync(request.Cwd, request.Budget, cts.Token); } catch (Exception ex) when (IsFailOpen(ex)) { diagnostic?.Invoke($"SessionStart scope resolution skipped: {ex.Message}"); - return SessionStartMemoryContextResult.Retry; + var flowsOnly = flowsTask is null ? null : await flowsTask; + var combined = Combine(null, [null, flowsOnly]); + + return combined.Disposition == SessionStartMemoryDisposition.Ready ? combined : SessionStartMemoryContextResult.Retry; } - // Started before any await so the lanes share the budget in parallel. + // Started before any further await so the lanes share the budget in parallel. var memoryTask = request.Disabled ? null : RunLaneAsync(() => memory.FetchWithScopeAsync(scope, request, cts.Token)); var guidelinesTask = request.GuidelinesDisabled ? null : RunLaneAsync(() => guidelines.FetchWithScopeAsync(scope, request, cts.Token)); - var flowsTask = request.FlowsDisabled ? null : RunLaneAsync(() => flows.FetchAsync(request, cts.Token)); var memoryResult = memoryTask is null ? null : await memoryTask; SessionStartMemoryContextResult?[] others = [ diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs index 3781bbd67c..cd9857b09b 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartFlowsLane.cs @@ -1,5 +1,6 @@ using System.Net; using System.Text; +using System.Text.Json; using System.Text.Json.Nodes; using Capacitor.Cli.Core.WorkItems; @@ -25,7 +26,14 @@ public async Task FetchAsync(SessionStartMemory if (outcome.Body is null) return new SessionStartMemoryContextResult(SessionStartMemoryDisposition.RetryableFailure, RetryAfter: outcome.RetryAfter); - return BuildFragment(JsonNode.Parse(outcome.Body)) is { } fragment + JsonNode? root; + try { + root = JsonNode.Parse(outcome.Body); + } catch (JsonException) { + return SessionStartMemoryContextResult.Empty; + } + + return BuildFragment(root) is { } fragment ? new SessionStartMemoryContextResult(SessionStartMemoryDisposition.Ready, fragment) : SessionStartMemoryContextResult.Empty; } diff --git a/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs b/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs index 45e3b64145..923e1052b5 100644 --- a/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs @@ -5,6 +5,9 @@ using Capacitor.Cli.Core.Harness; using Capacitor.Cli.Core; using Capacitor.Cli.PrDetection; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using WireMock.Server; namespace Capacitor.Cli.Tests.Unit.Harness.Antigravity; @@ -118,7 +121,7 @@ await Assert.That(SessionStartMemoryLifecyclePolicy.Decide(lifecycle)) public async Task Fetch_is_skipped_when_disabled_or_unscoped_or_out_of_budget() { // Each guard alone must suppress the fetch. A non-postable base url is checked BEFORE // any client is built: an unusable URL should spend no budget, no lease, and start no task. - // Both lanes off ⇒ suppressed; a single lane off would still fetch. + // All three lanes off ⇒ suppressed; a single lane on would still fetch. await Assert.That(await Hook().StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", "/repo", disabled: true, guidelinesDisabled: true, flowsDisabled: true, TimeSpan.FromSeconds(5))).IsNull(); @@ -134,6 +137,21 @@ await Assert.That(await Hook("").StartMemoryIndexTask( disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.FromSeconds(5))).IsNull(); } + [Test] + public async Task With_memory_and_guidelines_off_the_flows_lane_alone_still_fetches() { + using var server = WireMockServer.Start(); + server.Given(Request.Create().WithPath("/api/flows/definitions").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200).WithHeader("Content-Type", "application/json") + .WithBody("""{"definitions":[{"id":"code-review","offer":"proactive","when_to_use":"After a change is complete."}]}""")); + + var fragment = await Hook(server.Url!).StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", "/repo", + disabled: true, guidelinesDisabled: true, flowsDisabled: false, TimeSpan.FromSeconds(5)); + + await Assert.That(server.FindLogEntries(Request.Create().WithPath("/api/flows/definitions").UsingGet()).Count).IsEqualTo(1); + await Assert.That(fragment).IsNotNull(); + await Assert.That(fragment!).Contains("Flows you may offer"); + } + /// The memory subsystem is optional, so a store that cannot even be constructed must /// resolve to "no fragment" rather than fault the hook. A file where the store root's directory /// has to go makes construction throw synchronously, inside the try — the one failure mode that diff --git a/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs b/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs index fb11734ac6..e509cf3360 100644 --- a/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs @@ -113,7 +113,7 @@ public async Task Memory_contract_defaults_to_zero(string[] args, int expected) [Test] public async Task Memory_task_short_circuits_without_prerequisites() { // The url / scope / budget guards suppress even with guidelines ENABLED; disabled alone - // does not (a single lane off still fetches the other) — both off is required. + // does not (a single lane on still fetches) — all three off is required. await Assert.That(await Hook("not a url").StartMemoryIndexTask( "/abs/file.jsonl", "/scope", disabled: false, guidelinesDisabled: false, flowsDisabled: false, TimeSpan.FromSeconds(2), null)).IsNull(); diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs index 000d8edfd9..650febea8f 100644 --- a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs @@ -32,6 +32,15 @@ public async Task The_lane_is_off_for_a_harness_without_kcap_flows() { await Assert.That(SessionStartMemoryHookSupport.FlowsLaneDisabled(HarnessId.Codex, TestHarnesses.Under(new UserHome(home.Path)))).IsTrue(); } + [Test] + public async Task The_lane_is_on_for_a_harness_that_registers_kcap_flows() { + using var home = new TempDir(); + var config = home.CreateFile("config.toml", "[mcp_servers.kcap-flows]\ncommand = \"kcap\"\nargs = [\"mcp\", \"flows\"]\n"); + + await Assert.That(SessionStartMemoryHookSupport.FlowsLaneDisabled( + HarnessId.Codex, TestHarnesses.Under(new UserHome(home.Path)), config)).IsFalse(); + } + [Test] public async Task Proactive_flows_render_with_their_when_to_use() { var fragment = SessionStartFlowsLane.BuildFragment(JsonNode.Parse(Catalog)); @@ -75,6 +84,19 @@ public async Task Flows_past_the_caps_are_listed_by_id_only() { await Assert.That(fragment).EndsWith(SessionStartFlowsLane.Footer); } + [Test] + public async Task At_most_ten_flows_are_described_whatever_room_is_left() { + var defs = new JsonArray(); + for (var i = 1; i <= 12; i++) + defs.Add(new JsonObject { ["id"] = $"flow-{i:00}", ["offer"] = "proactive", ["when_to_use"] = "w" }); + + var lines = SessionStartFlowsLane.BuildFragment(new JsonObject { ["definitions"] = defs })!.Split('\n'); + + await Assert.That(lines.Count(l => l.StartsWith("- ", StringComparison.Ordinal))).IsEqualTo(10); + await Assert.That(lines.Where(l => l.StartsWith("- ", StringComparison.Ordinal)).Any(l => l.Contains("flow-11") || l.Contains("flow-12"))).IsFalse(); + await Assert.That(lines.Single(l => l.StartsWith("Also offerable: ", StringComparison.Ordinal))).IsEqualTo("Also offerable: flow-11, flow-12"); + } + [Test] public async Task A_huge_catalogue_still_fits_the_cap() { var defs = new JsonArray(); @@ -121,6 +143,13 @@ public async Task A_listing_without_guidance_fields_is_empty() { await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.CompleteWithoutContext); } + [Test] + public async Task A_non_json_200_is_empty_not_retried() { + var result = await Lane(HttpStatusCode.OK, "not json").FetchAsync(Req(), CancellationToken.None); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.CompleteWithoutContext); + } + [Test] public async Task The_lane_reads_the_definitions_listing() { var handler = new Handler(HttpStatusCode.OK, Catalog, null); @@ -165,6 +194,27 @@ public async Task Memory_comes_first_then_flows() { await Assert.That(fragment.IndexOf(SessionStartFlowsLane.Header, StringComparison.Ordinal)).IsGreaterThan(0); } + sealed class ThrowingScope : ISessionStartMemoryScopeResolver { + public Task ResolveAsync(string? cwd, TimeSpan budget, CancellationToken ct) => + throw new IOException("no scope"); + } + + [Test] + public async Task A_failing_scope_resolution_still_delivers_the_flows_lane() { + var time = new FakeTimeProvider(); + var scope = new FixedScope("repo", "machine"); + var composite = new SessionStartCompositeContextProvider(new ThrowingScope(), + new SessionStartMemoryContextProvider(scope, Lazy(new HttpClient(new Handler(HttpStatusCode.OK, "[]", null))), time), + new SessionStartGuidelinesLane(Lazy(new HttpClient(new Handler(HttpStatusCode.NoContent, "", null))), time), + new SessionStartFlowsLane(Lazy(new HttpClient(new Handler(HttpStatusCode.OK, Catalog, null))), time), + time); + + var result = await composite.GetAsync(Req(memory: true, guidelines: true, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(result.Fragment!).StartsWith(MemoryIndexEmitter.FragmentMarker + "\n" + SessionStartFlowsLane.Header); + } + static SessionStartCompositeContextProvider Composite( (HttpStatusCode, string) memory, (HttpStatusCode, string) guidelines, (HttpStatusCode, string) flows) { var scope = new FixedScope("repo", "machine"); From 4b1ca818bb1ec8e47db15c59f1fb47774df44879 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 16:26:14 +0200 Subject: [PATCH 12/16] Fix stale flow guidance docs, comments and skill wording Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- kcap/skills/agent-flows/SKILL.md | 6 +++--- src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs | 2 +- .../Instructions/AgentInstructionsWriterTests.cs | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 279fb86506..f8525acfe5 100644 --- a/README.md +++ b/README.md @@ -610,7 +610,7 @@ It provides these generic tools: - **`list_start_agent_options`** — read before `start_agent`: which of your daemons are connected on this machine (matched by the machine id a daemon has persisted here; nothing is written), how many agent slots each has free, and which harnesses each can start, plus the harness running the calling session. When no daemon runs on this machine it says so and names `kcap daemon start -d`, because `start_agent` would be refused. A daemon with `max_agents` 0 is shown with no limit. Called from a hosted agent, it says that every start runs on the daemon hosting that agent, which ignores `daemon`. Read-only. - **`start_agent`** — start one separate hosted agent in a git repository on this machine, and return at once. **It does not block and it does not poll**: unlike the flow tools above, it sends one request and answers `requested` as soon as the server has sent the launch command, before the agent registers, with the agent's id and the URL of its page. Required: `cwd` (an absolute directory inside the repository; a path inside a linked worktree resolves to the repository the worktree belongs to), `prompt` (the whole task, at most 16 384 bytes of UTF-8 — the agent shares none of the calling conversation) and `work_item` (a `wi:` or `le:` target key from the next-work feed, a work item id, `requester`, or `none`; there is no default). Optional: `vendor` (the harness to start; defaults to the harness running the session), `model`, `daemon` (only when several of your daemons run on this machine) and `session_id` (defaults to the calling session). The agent is an ordinary hosted agent that you supervise from the dashboard — unlike a harness's own subagent, it is not part of the calling session. The server tries once to attach its session to the named work item after that session starts, so nothing is attached when the call returns. A refusal comes back with the server's own reason: no daemon on this machine, the daemon at capacity, the limit of live agents, a start on the same key already in progress. When the server could not confirm that the launch command was sent, or does not answer within 60 seconds, the tool says the agent may or may not have started instead of reporting a failure. A server without the route, or with `Features:AgentStart` off, answers a bare 405 or an uncoded 404, and the tool says that the server cannot start agents. -**Offered flows.** When `kcap-flows` is registered for a harness, the SessionStart hook adds a short "Flows you may offer" block listing the catalogue flows marked `offer: proactive`, each with when to use it. The agent offers one at that moment and starts it only after you accept. The built-in `spec-review` and `code-review` flows ship as proactive; an admin controls any flow's offer through the `guidance` block of its definition. +**Offered flows.** When `kcap-flows` is registered for a harness, the SessionStart hook adds a short "Flows you may offer" block listing the catalogue flows marked `offer: proactive`, each with when to use it. The agent offers one at that moment and starts it only after you accept. The built-in `spec-review` and `code-review` flows ship as proactive; an admin controls any flow's offer through the `guidance` block of its definition. Against a server that predates flow guidance nothing is listed, and the built-ins are offered only by a server that ships their guidance. Every flow response reports which **workspace** the reviewer actually used, so you can tell whether it saw your uncommitted work: diff --git a/kcap/skills/agent-flows/SKILL.md b/kcap/skills/agent-flows/SKILL.md index 65d25782e8..0505182d69 100644 --- a/kcap/skills/agent-flows/SKILL.md +++ b/kcap/skills/agent-flows/SKILL.md @@ -17,7 +17,7 @@ description: >- # Agent Flows -Use the `kcap mcp flows` MCP tools (`list_flow_definitions`, `start_flow`, `send_to_participant`, `get_flow_status`, `close_flow`) to run a structured agent **flow**: your work is handed to a **separate, hosted participant agent** driven by a flow definition from the server's catalog, which returns a result (kind `findings` with the participant's result text, or `clean`); you address a `findings` result and keep iterating until the clean signal. This is a deliberate, heavier workflow — use it only when the user explicitly opts into it. +Use the `kcap mcp flows` MCP tools (`list_flow_definitions`, `get_flow_definition`, `start_flow`, `send_to_participant`, `get_flow_status`, `close_flow`) to run a structured agent **flow**: your work is handed to a **separate, hosted participant agent** driven by a flow definition from the server's catalog, which returns a result (kind `findings` with the participant's result text, or `clean`); you address a `findings` result and keep iterating until the clean signal. This is a deliberate, heavier workflow — use it when the user asks for a flow or accepts one you offered, never on your own initiative. ## Long rounds are normal @@ -41,10 +41,10 @@ Only start a flow when the user explicitly asks for one — e.g. "start a flow", ## Choosing the flow definition -Once the user has explicitly opted into a flow (see above), pick the `definition_id`: +Once the user has asked for a flow or accepted one you offered (see above), pick the `definition_id`: - The flow the user named, or the one you offered and they accepted. -- Anything else → the definition id the user named, or one from `list_flow_definitions` (read-only), which lists every definition this server can start — operator-published ones included — with its version, description, participant roles and their authored vendor and model, and whether it is single- or multi-participant. Call it whenever the user has not named a definition, or named one you have not seen listed: a definition it does not list is disabled, deleted or unknown, and `start_flow` will refuse it. If several fit, ask the user rather than guessing. A server that answers that it cannot list definitions only predates the tool — the two built-ins and any id the user names still work. +- If the user has not named one → choose from `list_flow_definitions` (read-only), which lists every definition this server can start — operator-published ones included — with its version, description, participant roles and their authored vendor and model, and whether it is single- or multi-participant. Call it whenever the user has not named a definition, or named one you have not seen listed: a definition it does not list is disabled, deleted or unknown, and `start_flow` will refuse it. If several fit, ask the user rather than guessing. A server that answers that it cannot list definitions only predates the tool — the two built-ins and any id the user names still work. 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 diff --git a/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs index a08f9f8de6..d4c18c0f53 100644 --- a/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs @@ -256,7 +256,7 @@ internal static SessionMemoryLifecycle LifecycleFor(string sessionId) => bool guidelinesDisabled, bool flowsDisabled, TimeSpan budget) { - // Both lanes off ⇒ nothing to fetch. A single disabled lane still runs the other. + // All three lanes off ⇒ nothing to fetch; any one enabled lane still runs. if ((disabled && guidelinesDisabled && flowsDisabled) || string.IsNullOrWhiteSpace(sessionId) || string.IsNullOrWhiteSpace(scopeRoot) || budget <= TimeSpan.Zero || !HookHttp.IsPostable(Url)) diff --git a/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs b/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs index 7066e5155f..c0656a1e7d 100644 --- a/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs +++ b/test/Capacitor.Cli.Core.Tests.Unit/Instructions/AgentInstructionsWriterTests.cs @@ -4,10 +4,10 @@ namespace Capacitor.Cli.Core.Tests.Unit.Instructions; public class AgentInstructionsWriterTests { [Test] - public async Task Body_carries_the_proactive_review_offer_nudge() { + public async Task Body_carries_the_flow_offer_nudge() { // The steering block is how the harnesses that do NOT consult a SKILL.md as an invocable - // skill (Copilot, Gemini, Pi, OpenCode, Antigravity) get the proactive review-offer - // behavior — dropping this paragraph silently removes the feature on those harnesses. + // skill (Copilot, Gemini, Pi, OpenCode, Antigravity) get the catalogue-flow offer + // behavior — dropping this paragraph silently removes it on those harnesses. await Assert.That(KcapAgentInstructions.Body).Contains("Flows you may offer"); await Assert.That(KcapAgentInstructions.Body).Contains("get_flow_definition"); await Assert.That(KcapAgentInstructions.Body).DoesNotContain("proactively OFFER an independent second-harness review"); From b02a65eda87ff886cf98274612e9094fdbc1fda3 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 16:42:38 +0200 Subject: [PATCH 13/16] Give the flows-lane hook test a round-trip budget Co-Authored-By: Claude Opus 5.5 --- .../Harness/Antigravity/AntigravitySessionStartMemoryTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs b/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs index 923e1052b5..1df09014c1 100644 --- a/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Harness/Antigravity/AntigravitySessionStartMemoryTests.cs @@ -144,8 +144,9 @@ public async Task With_memory_and_guidelines_off_the_flows_lane_alone_still_fetc .RespondWith(Response.Create().WithStatusCode(200).WithHeader("Content-Type", "application/json") .WithBody("""{"definitions":[{"id":"code-review","offer":"proactive","when_to_use":"After a change is complete."}]}""")); + // Budget is generous to accommodate the real round trip through scope resolution and the WireMock fetch under full parallelism. var fragment = await Hook(server.Url!).StartMemoryIndexTask("e80c33bfc10f4d2fb626b0043f488fc0", "/repo", - disabled: true, guidelinesDisabled: true, flowsDisabled: false, TimeSpan.FromSeconds(5)); + disabled: true, guidelinesDisabled: true, flowsDisabled: false, TimeSpan.FromSeconds(30)); await Assert.That(server.FindLogEntries(Request.Create().WithPath("/api/flows/definitions").UsingGet()).Count).IsEqualTo(1); await Assert.That(fragment).IsNotNull(); From 6a532c14de9da98c522be7211e4bd409200d58fc Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 18:02:36 +0200 Subject: [PATCH 14/16] Keep flows content from masking a scoped lane retry Co-Authored-By: Claude Opus 5.5 --- .../SessionStartCompositeContextProvider.cs | 60 ++++++++----- .../SessionStartMemory/FlowsLaneTests.cs | 87 +++++++++++++++++-- 2 files changed, 115 insertions(+), 32 deletions(-) diff --git a/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs b/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs index 9b7471267e..0d0813efbb 100644 --- a/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs +++ b/src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs @@ -20,31 +20,31 @@ public async Task GetAsync(SessionStartMemoryCo using var expiry = new CancellationTokenSource(request.Budget, time); using var cts = CancellationTokenSource.CreateLinkedTokenSource(request.CancellationToken, expiry.Token); - // The flows lane ignores scope, so it starts before scope resolves and survives its failure. var flowsTask = request.FlowsDisabled ? null : RunLaneAsync(() => flows.FetchAsync(request, cts.Token)); + // Flows ignore scope, and a slow scope resolution must not spend the budget they share. + if (request.Disabled && request.GuidelinesDisabled) + return Combine(null, null, flowsTask is null ? null : await flowsTask); + SessionStartMemoryScope scope; try { scope = await scopeResolver.ResolveAsync(request.Cwd, request.Budget, cts.Token); } catch (Exception ex) when (IsFailOpen(ex)) { diagnostic?.Invoke($"SessionStart scope resolution skipped: {ex.Message}"); - var flowsOnly = flowsTask is null ? null : await flowsTask; - var combined = Combine(null, [null, flowsOnly]); + if (flowsTask is not null) await flowsTask; - return combined.Disposition == SessionStartMemoryDisposition.Ready ? combined : SessionStartMemoryContextResult.Retry; + return SessionStartMemoryContextResult.Retry; } // Started before any further await so the lanes share the budget in parallel. var memoryTask = request.Disabled ? null : RunLaneAsync(() => memory.FetchWithScopeAsync(scope, request, cts.Token)); var guidelinesTask = request.GuidelinesDisabled ? null : RunLaneAsync(() => guidelines.FetchWithScopeAsync(scope, request, cts.Token)); - var memoryResult = memoryTask is null ? null : await memoryTask; - SessionStartMemoryContextResult?[] others = [ - guidelinesTask is null ? null : await guidelinesTask, - flowsTask is null ? null : await flowsTask - ]; + var memoryResult = memoryTask is null ? null : await memoryTask; + var guidelinesResult = guidelinesTask is null ? null : await guidelinesTask; + var flowsResult = flowsTask is null ? null : await flowsTask; - return Combine(memoryResult, others); + return Combine(memoryResult, guidelinesResult, flowsResult); } async Task RunLaneAsync(Func> lane) { @@ -56,31 +56,43 @@ async Task RunLaneAsync(Func().ToList(); + // Flows content alone must not commit the lease while memory or guidelines want a retry. + static SessionStartMemoryContextResult Combine( + SessionStartMemoryContextResult? memoryResult, SessionStartMemoryContextResult? guidelinesResult, SessionStartMemoryContextResult? flowsResult) { + var memoryFragment = Fragment(memoryResult); + var guidelinesFragment = Fragment(guidelinesResult); + var flowsFragment = Fragment(flowsResult); + + if (memoryFragment is not null || guidelinesFragment is not null) + return Committed(Compose(memoryFragment, [guidelinesFragment, flowsFragment])); - if (memoryFragment is not null || otherFragments.Count > 0) - return new SessionStartMemoryContextResult(SessionStartMemoryDisposition.Ready, Compose(memoryFragment, otherFragments)); + if (IsRetry(memoryResult) || IsRetry(guidelinesResult)) + return Retry([memoryResult, guidelinesResult, flowsResult]); - SessionStartMemoryContextResult?[] all = [memoryResult, ..others]; - var retries = all.Where(r => r?.Disposition == SessionStartMemoryDisposition.RetryableFailure).ToList(); - if (retries.Count == 0) return SessionStartMemoryContextResult.Empty; + if (flowsFragment is not null) return Committed(Compose(null, [flowsFragment])); + if (IsRetry(flowsResult)) return Retry([flowsResult]); - return new SessionStartMemoryContextResult( - SessionStartMemoryDisposition.RetryableFailure, RetryAfter: retries.Max(r => r!.RetryAfter)); + return SessionStartMemoryContextResult.Empty; - static string? Ready(SessionStartMemoryContextResult? r) => + static string? Fragment(SessionStartMemoryContextResult? r) => r is { Disposition: SessionStartMemoryDisposition.Ready, Fragment: { } f } ? f : null; + + static SessionStartMemoryContextResult Committed(string fragment) => new(SessionStartMemoryDisposition.Ready, fragment); + + static bool IsRetry(SessionStartMemoryContextResult? r) => r?.Disposition == SessionStartMemoryDisposition.RetryableFailure; + + static SessionStartMemoryContextResult Retry(SessionStartMemoryContextResult?[] all) => + new(SessionStartMemoryDisposition.RetryableFailure, RetryAfter: all.Where(IsRetry).Max(r => r!.RetryAfter)); } /// Marker first: Pi and OpenCode capture stdout only when it opens with it, and only the memory /// fragment carries it. - static string Compose(string? memoryFragment, IReadOnlyList others) { - var rest = string.Join("\n\n", others); + static string Compose(string? memoryFragment, IReadOnlyList others) { + var present = others.OfType().ToList(); + var rest = string.Join("\n\n", present); if (memoryFragment is null) return MemoryIndexEmitter.FragmentMarker + "\n" + rest; - return others.Count == 0 ? memoryFragment : memoryFragment + "\n\n" + rest; + return present.Count == 0 ? memoryFragment : memoryFragment + "\n\n" + rest; } static bool IsFailOpen(Exception ex) => diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs index 650febea8f..477b1777bb 100644 --- a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FlowsLaneTests.cs @@ -199,17 +199,88 @@ public Task ResolveAsync(string? cwd, TimeSpan budget, throw new IOException("no scope"); } - [Test] - public async Task A_failing_scope_resolution_still_delivers_the_flows_lane() { - var time = new FakeTimeProvider(); - var scope = new FixedScope("repo", "machine"); - var composite = new SessionStartCompositeContextProvider(new ThrowingScope(), - new SessionStartMemoryContextProvider(scope, Lazy(new HttpClient(new Handler(HttpStatusCode.OK, "[]", null))), time), - new SessionStartGuidelinesLane(Lazy(new HttpClient(new Handler(HttpStatusCode.NoContent, "", null))), time), + sealed class HangingScope : ISessionStartMemoryScopeResolver { + public int Calls; + + public Task ResolveAsync(string? cwd, TimeSpan budget, CancellationToken ct) { + Calls++; + return new TaskCompletionSource().Task; + } + } + + static SessionStartCompositeContextProvider WithScope(ISessionStartMemoryScopeResolver resolver, (HttpStatusCode, string) memory, (HttpStatusCode, string) guidelines) { + var time = new FakeTimeProvider(); + + return new SessionStartCompositeContextProvider(resolver, + new SessionStartMemoryContextProvider(new FixedScope("repo", "machine"), Lazy(new HttpClient(new Handler(memory.Item1, memory.Item2, null))), time), + new SessionStartGuidelinesLane(Lazy(new HttpClient(new Handler(guidelines.Item1, guidelines.Item2, null))), time), new SessionStartFlowsLane(Lazy(new HttpClient(new Handler(HttpStatusCode.OK, Catalog, null))), time), time); + } + + const string MemoryBody = """[{"memory_id":"m1","slug":"s","audience":"org","description":"d","kind":"preference"}]"""; + + [Test] + public async Task A_failing_scope_resolution_still_delivers_the_flows_lane_when_scoped_lanes_are_off() { + var composite = WithScope(new ThrowingScope(), (HttpStatusCode.OK, "[]"), (HttpStatusCode.NoContent, "")); + + var result = await composite.GetAsync(Req(flows: true)); - var result = await composite.GetAsync(Req(memory: true, guidelines: true, flows: true)); + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(result.Fragment!).StartsWith(MemoryIndexEmitter.FragmentMarker + "\n" + SessionStartFlowsLane.Header); + } + + [Test] + public async Task A_failing_scope_resolution_retries_while_memory_is_enabled() { + var composite = WithScope(new ThrowingScope(), (HttpStatusCode.OK, "[]"), (HttpStatusCode.NoContent, "")); + + var result = await composite.GetAsync(Req(memory: true, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.RetryableFailure); + } + + [Test] + public async Task A_flows_only_request_never_resolves_scope() { + var scope = new HangingScope(); + var composite = WithScope(scope, (HttpStatusCode.OK, "[]"), (HttpStatusCode.NoContent, "")); + + var result = await composite.GetAsync(Req(flows: true)).WaitAsync(TimeSpan.FromSeconds(10)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(result.Fragment!).Contains(SessionStartFlowsLane.Header); + await Assert.That(scope.Calls).IsEqualTo(0); + } + + [Test] + public async Task Flows_content_does_not_mask_a_memory_retry() { + var result = await Composite(memory: (HttpStatusCode.ServiceUnavailable, ""), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.RetryableFailure); + await Assert.That(result.Fragment).IsNull(); + } + + [Test] + public async Task Flows_content_does_not_mask_a_guidelines_retry() { + var result = await Composite(memory: (HttpStatusCode.OK, "[]"), guidelines: (HttpStatusCode.ServiceUnavailable, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(guidelines: true, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.RetryableFailure); + } + + [Test] + public async Task Memory_content_commits_even_when_guidelines_want_a_retry() { + var result = await Composite(memory: (HttpStatusCode.OK, MemoryBody), guidelines: (HttpStatusCode.ServiceUnavailable, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, guidelines: true, flows: true)); + + await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); + await Assert.That(result.Fragment!).Contains(SessionStartFlowsLane.Header); + } + + [Test] + public async Task Empty_memory_and_guidelines_still_deliver_flows_under_the_marker() { + var result = await Composite(memory: (HttpStatusCode.OK, "[]"), guidelines: (HttpStatusCode.NoContent, ""), flows: (HttpStatusCode.OK, Catalog)) + .GetAsync(Req(memory: true, guidelines: true, flows: true)); await Assert.That(result.Disposition).IsEqualTo(SessionStartMemoryDisposition.Ready); await Assert.That(result.Fragment!).StartsWith(MemoryIndexEmitter.FragmentMarker + "\n" + SessionStartFlowsLane.Header); From 8682ddbb920b360fc2b6cf62c46eb6bb60b8d0f5 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 18:03:02 +0200 Subject: [PATCH 15/16] Scope guide lookup to catalogue flows and add it to review-flows Co-Authored-By: Claude Opus 5.5 --- kcap/skills/agent-flows/SKILL.md | 6 +++--- kcap/skills/review-flows/SKILL.md | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/kcap/skills/agent-flows/SKILL.md b/kcap/skills/agent-flows/SKILL.md index 0505182d69..b71571038a 100644 --- a/kcap/skills/agent-flows/SKILL.md +++ b/kcap/skills/agent-flows/SKILL.md @@ -87,7 +87,7 @@ server rejects the override outright. ## Read the flow's guide first -Call `get_flow_definition(definition_id)` before `start_flow`. The guide is written by whoever published the flow: what to put in `context`, which target to name, how to iterate on its results and when to close. Where it differs from the generic rules below, follow the guide. A server that does not publish guides says so; then work from the definition's description and the rules below. +Before a catalogue start (`definition_id`), call `get_flow_definition(definition_id)`. The guide is written by whoever published the flow: what to put in `context`, which target to name, how to iterate on its results and when to close. Where it differs from the generic rules below, follow the guide. A server that does not publish guides says so; then work from the definition's description and the rules below. A flow started with an inline `definition_yaml` has no catalogue id and no guide: drive it by the generic rules and its own YAML. ## Composing a dynamic flow @@ -171,7 +171,7 @@ The server enforces per-run budgets; watch for these in tool error responses: ## Workflow -Every flow starts the same way: +A catalogue flow (`definition_id`) starts with its guide; an inline `definition_yaml` flow skips this step: ``` get_flow_definition(definition_id) @@ -226,7 +226,7 @@ report completion to user | Tool | Required args | Optional args | When to call | |---|---|---|---| | `list_flow_definitions` | — | — | Before `start_flow`, whenever the user has not named a definition or named one you have not seen listed. Read-only: returns each runnable definition's id, version, description, whether it is single- or multi-participant, and its participants' role, authored vendor and model. A `server_catching_up` error is retryable — do not read it as an empty catalog. | -| `get_flow_definition` | `definition_id` | — | Before `start_flow`, every time. Read-only: returns the definition's participants and when to use it, then its authored driver guide (what to submit, how to iterate, when to close). An unknown, disabled or deleted id is an error; an older server reports that it publishes no guides. | +| `get_flow_definition` | `definition_id` | — | Before every catalogue (`definition_id`) `start_flow`. Read-only: returns the definition's participants and when to use it, then its authored driver guide (what to submit, how to iterate, when to close). An unknown, disabled or deleted id is an error; an older server reports that it publishes no guides. | | `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` (omit to read the newest open flow this session started, or on a harness without a session identity the newest one started from this workspace; several open flows are listed instead), `session_id` (look up another session's flows; defaults to this session), `wait` (`true`/`false`, defaults to `false`) — when `true`, blocks until the round is terminal or roughly 3.5 minutes pass, instead of returning the current snapshot immediately | Poll or check the current status of a flow run (running, waiting, completed, failed). Use `wait: true` to ride out a long round instead of polling repeatedly yourself, and omit `flow_run_id` to recover a flow whose id you never received or lost. | diff --git a/kcap/skills/review-flows/SKILL.md b/kcap/skills/review-flows/SKILL.md index 9c06df586a..98ad4c5303 100644 --- a/kcap/skills/review-flows/SKILL.md +++ b/kcap/skills/review-flows/SKILL.md @@ -157,6 +157,9 @@ After applying the role-surface safety gate, if `start_review_flow` / `submit_re ## Workflow ``` +get_flow_definition(kind) + → follow its guide: what to submit, how to iterate (a server that publishes no guides says so; the rest of this skill applies) + start_review_flow(kind, target_kind, target_ref, target_title, context) → reviewer returns a result: findings (with the findings text) | clean From bdd2dd6de5d3718a70ed3a6b991edb97a5f694c8 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sat, 3 Oct 2026 18:05:36 +0200 Subject: [PATCH 16/16] Split session-start test stubs into one type per file Co-Authored-By: Claude Opus 5.5 --- .../SessionStartMemory/FixedScope.cs | 8 ++++++++ .../{SessionStartStubs.cs => Handler.cs} | 6 ------ 2 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FixedScope.cs rename test/Capacitor.Cli.Tests.Unit/SessionStartMemory/{SessionStartStubs.cs => Handler.cs} (71%) diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FixedScope.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FixedScope.cs new file mode 100644 index 0000000000..b77942e397 --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/FixedScope.cs @@ -0,0 +1,8 @@ +using Capacitor.Cli.SessionStartMemory; + +namespace Capacitor.Cli.Tests.Unit.SessionStartMemory; + +sealed class FixedScope(string? repo, string? machine) : ISessionStartMemoryScopeResolver { + public Task ResolveAsync(string? cwd, TimeSpan budget, CancellationToken ct) => + Task.FromResult(new SessionStartMemoryScope(repo, machine)); +} diff --git a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/Handler.cs similarity index 71% rename from test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs rename to test/Capacitor.Cli.Tests.Unit/SessionStartMemory/Handler.cs index 87cb844f27..8066a138e3 100644 --- a/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartStubs.cs +++ b/test/Capacitor.Cli.Tests.Unit/SessionStartMemory/Handler.cs @@ -1,15 +1,9 @@ using System.Net; using System.Net.Http.Headers; using System.Text; -using Capacitor.Cli.SessionStartMemory; namespace Capacitor.Cli.Tests.Unit.SessionStartMemory; -sealed class FixedScope(string? repo, string? machine) : ISessionStartMemoryScopeResolver { - public Task ResolveAsync(string? cwd, TimeSpan budget, CancellationToken ct) => - Task.FromResult(new SessionStartMemoryScope(repo, machine)); -} - sealed class Handler(HttpStatusCode status, string body, TimeSpan? retryAfter = null) : HttpMessageHandler { public int Calls; public string? Uri;