Make the kcap agent commands flow-participant aware

README.md

@@ -948,13 +948,20 @@ kcap agent start claude -d                    # start without attaching; prints
 
 ```bash
 kcap agent                 # no subcommand — same as `kcap agent ls`
-kcap agent ls              # list daemon-hosted agents (id, status, repo)
+kcap agent ls              # list daemon-hosted agents (id, status, kind, repo)
 kcap agent attach ab12     # re-attach your terminal (any unique id prefix works)
 kcap agent stop ab12       # graceful /exit, then terminate
 kcap agent stop --all -y   # stop every agent this daemon hosts, no prompt
 ```
 
-Agent ids are long, so `attach` and `stop` accept **any unique prefix** — an ambiguous one lists the candidates instead of guessing. `stop --all` includes `--private` agents and prompts for confirmation unless you pass `--yes`/`-y`. `--all` stops **every** agent this daemon hosts, not just the ones you started from this CLI — including agents launched from the web UI and review-flow participants (e.g. reviewers mid-round). Making these commands flow-participant aware is tracked in [#379](https://github.com/kurrent-io/kcap-cli/issues/379). If a stop can't be confirmed (e.g. an agent that never exits, even after SIGKILL), `stop` prints a per-agent "Failed to stop `<id>` — see `kcap daemon logs`." line and exits non-zero; `stop --all` still stops the rest and reports each agent's outcome individually.
+Agent ids are long, so `attach` and `stop` accept **any unique prefix** — an ambiguous one lists the candidates instead of guessing. `stop --all` includes `--private` agents and prompts for confirmation unless you pass `--yes`/`-y`; a stop that cannot be confirmed prints a per-agent failure line and exits non-zero.
+
+**Agents that aren't yours.** `kcap agent ls` shows a `KIND` column: `agent` for ones you started, `review` for PR-review agents, and `review-flow` for review-flow participants (with their role). The daemon protects the latter two, because they are driven by the flow protocol rather than by you:
+
+- `kcap agent attach` on one is **read-only** — you see its output, your keystrokes are not delivered, and your terminal size is not applied to it.
+- `kcap agent stop` on one is **refused** unless you pass `--force`, and `stop --all` skips them and says how many it skipped.
+
+Enforcement lives in the daemon, so a current CLI can't bypass it by skipping the check or lying about `--force`. That guarantee has two version-skew exceptions: an old `kcap` sends the legacy `Stop` request, which has no `--force` concept — the daemon treats that as `--force`, so an old client can silently force-stop a review or review-flow agent. And against an old daemon, `ls`/`attach` degrade silently (no kind reported, every agent reads as `agent`), but `stop` doesn't degrade — it sends a newer request format the old daemon can't decode, so the connection closes and the CLI tells you to restart the daemon instead of stopping anything.
 
 `agent start` auto-starts the daemon if one isn't already running, and needs a configured server for the daemon to record to. `ls`, `attach`, and `stop` only talk to the local socket, so they work without one. A locally-started agent appears in **your own** web UI (owner-only until you share it from the web UI); use `--private` to opt out of registration entirely. Unix only for now.
 

docs/superpowers/specs/2026-07-29-ai1557-flow-participant-aware-agent-commands-design.md

@@ -0,0 +1,147 @@
+# Flow-participant-aware `kcap agent` commands — design
+
+Date: 2026-07-29
+Status: proposed
+Issue: AI-1557 / [#379](https://github.com/kurrent-io/kcap-cli/issues/379)
+
+## Motivation
+
+`kcap agent ls|attach|stop` treats every agent the daemon hosts identically.
+`HandleLocalListAsync` returns all of `_agents` unfiltered and `HandleLocalAttachAsync` attaches
+to any id in it, so an agent the daemon launched as a **review-flow participant** — a reviewer
+mid-round — is indistinguishable from one the user started with `kcap agent start`.
+
+Two concrete failures follow:
+
+1. **Attach injects turns into a flow.** Participants are addressed through the flow protocol
+   (`send_to_participant`), never by typing at them. `kcap agent attach <reviewer-id>` hands the
+   user raw PTY stdin, with no round, no routing, and no record of the interjection.
+2. **Stop kills a participant silently.** `kcap agent stop --all` stops every agent in the map.
+   A reviewer vanishes mid-round and the flow has no idea why.
+
+The second one arrived with `agent stop`, which is why this was deferred rather than solved in
+the same change: the read path was already blind, and the write path made the blindness bite.
+
+The daemon already has everything needed. `AgentInstance` carries `Kind`
+(`LaunchKind.Default`/`Review`/`ReviewFlow`), plus `FlowRunId`/`FlowRole` for flow launches. None
+of it reaches the CLI.
+
+## Goal
+
+The CLI can tell a review or flow agent from the user's own, shows it, and refuses to mutate it
+by accident.
+
+## Decisions
+
+1. **The protected set is `Kind != Default`** — `ReviewFlow` participants and `Review` agents.
+   Plain web-UI-launched agents (`Default`, not locally spawned) stay unprotected: they are the
+   user's own work by another route, and protecting them would make `stop --all` nearly useless.
+
+2. **Attach to a protected agent is read-only**, not refused. Watching a reviewer work is a real
+   debugging need with no other local equivalent, and read-only preserves the flow invariant
+   exactly — output flows out, nothing flows in.
+
+3. **A read-only viewer does not resize the PTY.** It is never added to
+   `AgentInstance.ClientDims`, so it cannot shrink a participant's terminal through the
+   min-clamp. The cost lands on the observer: if their terminal differs from the participant's,
+   their own view wraps or clips. That is the correct side to put it on.
+
+4. **`stop` on a protected agent refuses unless `--force`.** `stop --all` stops only unprotected
+   agents and *states* what it skipped — a silent omission reads as "stopped everything".
+
+5. **Enforcement is daemon-side, not client-side.** The daemon owns the protection for both
+   attach and stop, so a stale or hand-rolled client cannot bypass it, and the two subcommands
+   share one model. This costs a wire change that a client-side filter would not.
+
+6. **Against an older daemon, no protection applies.** Accepted, not mitigated — see Version
+   skew.
+
+## Wire protocol
+
+`FrameType` is append-only. One new value:
+
+| Value | Direction | Payload |
+|---|---|---|
+| `StopV2 = 10` | client → daemon | `mode⇥agentId` — mode `normal`\|`force`; empty id = all eligible |
+| `AttachedReadOnly = 71` | daemon → client | `lp(agentId) + lp(reason) + snapshot` |
+
+Two payload extensions to existing frames:
+
+- **`AgentList`** rows become `id⇥status⇥repo⇥kind⇥flowRunId⇥flowRole`. `kind` is
+  `agent`\|`review`\|`review-flow`; the flow fields are empty for non-flow agents.
+- **`StopAck`** rows keep the `id⇥status` shape, with `status` gaining `skipped` alongside the
+  existing `stopped`\|`failed`.
+
+**Why `AttachedReadOnly` is a new frame rather than a flag on `Attached`.** `Attached`'s payload
+is `lp(agentId) + snapshot`, and the decoder returns `f.Bytes[o..]` — the snapshot is *everything
+remaining*. A trailing flag would be swallowed into the replay buffer and painted onto the user's
+terminal. The `isPrivate` trick `FrameCodec.Spawn` uses works only because Spawn's payload is
+fully counted, so it does not transfer. A separate frame also fails closed: an older CLI cannot
+decode type 71, so it errors out instead of silently pumping stdin at a participant.
+
+`Stop = 8` and `Attached = 64` stay decodable so nothing regresses mid-upgrade; the CLI sends
+`StopV2` and the daemon sends `AttachedReadOnly` only for protected agents.
+
+## Daemon changes
+
+- `HandleLocalListAsync` emits the three new columns from `Kind`/`FlowRunId`/`FlowRole`.
+- `HandleLocalAttachAsync` resolves the agent's `Kind`. For a protected agent it runs
+  `AttachClientLoopAsync` in read-only mode: the sink is registered as today, the `Stdin` arm is
+  dropped, the `Resize` arm is dropped, and the client is never entered into `ClientDims`.
+- A new `HandleLocalStopV2Async(mode, agentId, …)` applies decision 4:
+  - explicit id, protected, `normal` → `Error` naming the kind and flow;
+  - explicit id, protected, `force` → stop;
+  - empty id, `normal` → stop unprotected agents; protected ones appear in the ack as `skipped`;
+  - empty id, `force` → stop everything, as `stop --all` does today.
+- `LocalControlServer` routes `FrameType.StopV2`.
+
+The `IsPrivate` guard on `HandleStopAgent` and the server-origin path are untouched.
+
+## Client changes
+
+- `AgentRow` gains `Kind`, `FlowRunId`, `FlowRole`. The parser accepts **3 or more** columns and
+  defaults the new ones, so a running older daemon still lists correctly.
+- `ls` renders a `KIND` column, with the flow role appended for flow agents.
+- `attach` prints the read-only banner on `AttachedReadOnly`, and skips its own stdin pump, its
+  resize pump, and the `SizeFrame()` nudge it normally sends on attach. All three are cosmetic —
+  the daemon ignores them for a protected agent — but sending them would be misleading.
+- `stop` accepts `--force`, sends `StopV2`, and reports `skipped` rows distinctly from
+  `stopped`. With `--all --force`, the confirmation prompt lists protected agents in their own
+  labelled group, so the blast radius is visible before the user answers.
+
+## Version skew
+
+An older daemon replies with three-column `AgentList` rows, so every agent reads as `agent` and
+**no protection engages** — `stop --all` behaves exactly as it does today.
+
+This is deliberately not mitigated. A three-column reply from an old daemon is indistinguishable
+from a new daemon reporting three unprotected agents, so there is nothing to detect without
+adding a version probe. The window closes on its own: a daemon self-restarts onto the new binary
+when idle after `kcap update`. `StopV2` is the one place skew is visible, and it already
+surfaces through the existing "this daemon is too old for `agent stop`" message.
+
+## Documentation
+
+- `help-agent.txt` — read-only attach, `stop --force`, and what `stop --all` skips.
+- `README.md` — the `### Local agents (kcap agent)` section. Its current `--all` warning
+  (added when this gap was deferred, pointing at #379) is replaced by the real behaviour.
+
+## Testing
+
+- Daemon: `SeedAgentForTest` already takes `kind`/`flowRunId`/`flowRole`, so protection tests are
+  cheap. Cover — protected attach drops stdin and resize and never enters `ClientDims`;
+  protected stop without force yields `Error`; with force stops; stop-all marks protected rows
+  `skipped` and stops the rest.
+- Client: `AgentRow` parsing for 3-column (old daemon) and 6-column rows; `ls` rendering;
+  `skipped` rows reported distinctly.
+- `AgentVerbDispatchTests` gains a case pinning that a protected agent refuses `stop` without
+  `--force`.
+
+## Out of scope
+
+- **The `--all` confirmation TOCTOU** — the client lists and prompts, then the daemon
+  re-enumerates, so an agent launched during the prompt is stopped unlisted. Tracked separately;
+  this design narrows but does not close it.
+- **Telling the flow that a participant was force-stopped.** The flow still sees a participant
+  vanish; attributing that needs server-side work.
+- **Plain web-UI-launched agents**, per decision 1.

src/Capacitor.Cli.Core/LocalIpc/FrameCodec.cs

@@ -50,7 +50,8 @@ static async Task<bool> ReadExactlyOrEof(Stream s, byte[] buf, CancellationToken
         FrameType.Error or FrameType.Attach or FrameType.AgentList
             or FrameType.Restart or FrameType.RestartAck
             or FrameType.Stop or FrameType.StopAck => Encoding.UTF8.GetBytes(f.Text),
-        FrameType.Attached or FrameType.Spawn => f.Bytes, // pre-encoded by Attached(...)/Spawn(...)
+        FrameType.Attached or FrameType.Spawn
+            or FrameType.StopV2 or FrameType.AttachedReadOnly => f.Bytes, // pre-encoded by the helpers below
         _ => throw new InvalidDataException($"unencodable frame {f.Type}"),
     };
 
@@ -62,7 +63,8 @@ or FrameType.Restart or FrameType.RestartAck
         FrameType.Error or FrameType.Attach or FrameType.AgentList
             or FrameType.Restart or FrameType.RestartAck
             or FrameType.Stop or FrameType.StopAck => new(t) { Text = Encoding.UTF8.GetString(p) },
-        FrameType.Attached or FrameType.Spawn => new(t) { Bytes = p },
+        FrameType.Attached or FrameType.Spawn
+            or FrameType.StopV2 or FrameType.AttachedReadOnly => new(t) { Bytes = p },
         _ => throw new InvalidDataException($"undecodable frame {t}"),
     };
 
@@ -125,6 +127,36 @@ public static (string agentId, byte[] snapshot) Attached(LocalFrame f) {
         return (id, f.Bytes[o..]);
     }
 
+    // --- StopV2 structured payload ---
+    public static LocalFrame StopV2(bool force, string agentId) {
+        using var ms = new MemoryStream();
+        ms.WriteByte((byte)(force ? 1 : 0));
+        WriteLp(ms, agentId);
+        return new(FrameType.StopV2) { Bytes = ms.ToArray(), Text = agentId };
+    }
+    public static (bool force, string agentId) StopV2(LocalFrame f) {
+        Require(f.Bytes, 0, 1);
+        var o = 1;
+        return (f.Bytes[0] == 1, ReadLp(f.Bytes, ref o));
+    }
+
+    // --- AttachedReadOnly structured payload ---
+    // Length-prefixed id AND reason before the snapshot: the snapshot is the unbounded tail, so
+    // anything appended after it would be painted onto the user's terminal instead of parsed.
+    public static LocalFrame AttachedReadOnly(string agentId, string reason, byte[] snapshot) {
+        using var ms = new MemoryStream();
+        WriteLp(ms, agentId);
+        WriteLp(ms, reason);
+        ms.Write(snapshot);
+        return new(FrameType.AttachedReadOnly) { Bytes = ms.ToArray(), Text = agentId };
+    }
+    public static (string agentId, string reason, byte[] snapshot) AttachedReadOnly(LocalFrame f) {
+        var o = 0;
+        var id = ReadLp(f.Bytes, ref o);
+        var reason = ReadLp(f.Bytes, ref o);
+        return (id, reason, f.Bytes[o..]);
+    }
+
     // --- byte helpers ---
     static byte[] Dims(ushort c, ushort r) { var b = new byte[4]; BinaryPrimitives.WriteUInt16BigEndian(b, c); BinaryPrimitives.WriteUInt16BigEndian(b.AsSpan(2), r); return b; }
     static byte[] BeInt(int v) { var b = new byte[4]; BinaryPrimitives.WriteInt32BigEndian(b, v); return b; }

src/Capacitor.Cli.Core/LocalIpc/FrameType.cs

@@ -12,12 +12,14 @@ public enum FrameType : byte {
     List    = 6,   // request the daemon's agent list (for `kcap agent ls`)
     Restart = 7,   // request restart-after-update (Text = "when-idle"|"now"|"force")
     Stop    = 8,   // stop an agent (Text = agent id; empty = every agent this daemon hosts)
+    StopV2  = 10,  // stop with a force flag (see FrameCodec.StopV2); supersedes Stop
     // daemon → client
     Attached  = 64,
     Stdout    = 65,
     Exited    = 66,
     Error     = 67,
-    AgentList = 68, // UTF-8 table payload: one `id\tstatus\tcwd` line per agent
+    AgentList = 68, // UTF-8 table payload: one `id\tstatus\trepo\tkind\tflowRunId\tflowRole` line per agent
     RestartAck = 69, // acknowledgement for Restart (Text = short status)
-    StopAck    = 70, // acknowledgement for Stop (Text = one `id\tstatus` line per agent; status is "stopped" or "failed")
+    StopAck    = 70, // acknowledgement for Stop (Text = one `id\tstatus` line per agent; status is "stopped", "skipped", or "failed")
+    AttachedReadOnly = 71, // Attached for a protected agent: id + reason + snapshot, no input accepted
 }

src/Capacitor.Cli.Core/LocalIpc/LocalFrame.cs

@@ -22,4 +22,5 @@ public sealed record LocalFrame(FrameType Type) {
     public static LocalFrame RestartAck(string status)  => new(FrameType.RestartAck) { Text = status };
     public static LocalFrame Stop(string agentId)       => new(FrameType.Stop)       { Text = agentId };
     public static LocalFrame StopAck(string payload)    => new(FrameType.StopAck)    { Text = payload };
+    public static LocalFrame StopV2(bool force, string agentId) => FrameCodec.StopV2(force, agentId);
 }

src/Capacitor.Cli.Core/Resources/help-agent.txt

@@ -5,10 +5,12 @@ Usage: kcap agent <subcommand>
 Subcommands:
   start <vendor> [flags] [-- <agent args>]
                           Start an agent the daemon hosts for you
-  ls                      List this daemon's agents (id, status, repo)
+  ls                      List this daemon's agents (id, status, kind, repo)
   attach <agent-id>       Attach your terminal to a running agent
-  stop <agent-id>         Stop an agent (graceful /exit, then terminate)
-  stop --all [-y]         Stop every agent this daemon hosts
+  stop <agent-id> [--force]
+                          Stop an agent (graceful /exit, then terminate)
+  stop --all [-y] [--force]
+                          Stop every agent this daemon hosts
 
 Running `kcap agent` with no subcommand lists agents.
 
@@ -22,21 +24,28 @@ Options for start:
   --                      Everything after this is passed to the vendor CLI verbatim
 
 Options for stop:
-  --all                   Stop every agent, including --private ones
+  --all                   Stop every agent, including --private ones. Review and
+                          review-flow agents are skipped unless --force is given.
+  --force                 Stop a single review or review-flow agent that would
+                          otherwise be refused, or include them under --all.
+                          Stopping a flow participant mid-round leaves the flow
+                          without it.
   --yes, -y               Skip the confirmation prompt for --all
 
-  --all stops every agent this daemon hosts, not just the ones you started
-  from this CLI — including agents launched from the web UI and review-flow
-  participants (e.g. reviewers mid-round). See
-  https://github.com/kurrent-io/kcap-cli/issues/379.
-
 Options for ls, attach, stop:
   --daemon <name>         Target a specific daemon (defaults to your profile's)
 
 Agent ids:
   Any unique prefix works — `kcap agent attach ab12` is enough as long as it
   matches exactly one agent. An ambiguous prefix lists the candidates.
 
+Review and flow agents:
+  Agents the daemon runs as part of a review flow (KIND `review-flow`) or a PR
+  review (`review`) are not yours to drive directly — they are addressed through
+  the flow tools. `attach` gives you a read-only view of one: you see its output,
+  your input is not delivered, and your terminal size is not applied to it.
+  `stop` refuses unless you pass --force.
+
 Notes:
   The daemon owns the agent, not your terminal, so you can detach and the agent
   keeps running. Detach with the prefix key Ctrl-Q then d, and reattach later

src/Capacitor.Cli.Core/Resources/help-usage.txt

@@ -28,9 +28,9 @@ Configuration:
 
 Agents:
   agent start <vendor> [-d] [--worktree] [--private] [-- <args>]  Start a daemon-hosted agent
-  agent ls                         List this daemon's agents (id, status, repo)
+  agent ls                         List this daemon's agents (id, status, kind, repo)
   agent attach <id>                Attach your terminal to a running agent (id prefix ok)
-  agent stop <id> | --all [-y]     Stop one agent, or all of them
+  agent stop <id> | --all [-y] [--force]  Stop one agent, or all of them
 
 Daemon:
   daemon start [-d] [--name N]     Start the daemon (foreground, or -d for background; refuses on duplicate name)