Repository navigation
Group the agent commands under kcap agent - #383
Conversation
Regroups run-agent/attach/ls under `kcap agent start|ls|stop|attach`, adds a local stop path, and renames --name to --daemon. Refs AI-1555. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also clarifies in the spec that a local stop falls back to the PID-record reap before reporting an unknown agent. Refs AI-1555. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…name Comments and a user-visible overflow-detach message still referenced the old kcap run-agent/attach/ls surface. Update them to kcap agent start/attach/ls/stop so no stale command names remain in src/.
Threads the resolved daemon name through the version-skew hint so it suggests `kcap daemon restart --force --name <name>` instead of a bare restart the daemon would refuse while busy stopping agents. Distinguishes a non-answering or erroring daemon from a genuinely empty agent list in FetchAgentsAsync, so `stop --all` no longer silently no-ops when the daemon never replies. Documents that `stop --all` affects every agent the daemon hosts, not just ones started locally (#379). Lowercases full agent ids on the pass-through path so uppercase ids resolve like their prefixes do, and rejects a positional id combined with --all instead of ignoring the id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoGroup agent lifecycle commands under
AI Description
Diagram
High-Level Assessment
Files changed (23)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| case "agent": | ||
| return await AgentCommand.HandleAsync(args); |
There was a problem hiding this comment.
2. Getting started remains unsynchronized 📘 Rule violation ⚙ Maintainability
The PR replaces three top-level commands with the kcap agent group, but the README Getting started section was not updated to introduce or link to the changed command surface. This fails the checklist's explicit requirement to update both Getting started and the applicable command section.
Agent Prompt
## Issue description
The new `kcap agent` command surface is documented in the command reference but not in README.md's Getting started section.
## Issue Context
PR Compliance ID 7 explicitly requires user-facing CLI changes to update both the Getting started section and the applicable per-command section.
## Fix Focus Areas
- README.md[49-132]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| } | ||
| } | ||
|
|
||
| target = ""; |
There was a problem hiding this comment.
5. Confirmation omits stopped agents 🐞 Bug ≡ Correctness
StopAsync confirms a client-side agent snapshot but sends an empty target that the daemon expands into a new snapshot at execution time. An agent launched between the prompt and stop-frame handling is therefore terminated without appearing in the confirmation list.
Agent Prompt
## Issue description
The `--all` confirmation covers one snapshot, while the empty stop sentinel operates on a later daemon-side snapshot. Concurrently launched agents can be stopped without being shown to the user.
## Issue Context
Bind execution to the exact IDs displayed during confirmation, such as by sending those IDs explicitly or introducing a versioned snapshot that the daemon validates atomically.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/AgentCommand.cs[113-145]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs[21-27]
- src/Capacitor.Cli.Core/LocalIpc/LocalFrame.cs[23-24]
- test/Capacitor.Cli.Tests.Unit/AgentOrchestratorLocalAttachTests.cs[620-633]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4ad267c62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static string? NameFrom(string[] args) { | ||
| var i = Array.IndexOf(args, "--daemon"); | ||
|
|
||
| return i >= 0 && i + 1 < args.Length ? args[i + 1] : null; |
There was a problem hiding this comment.
When --daemon is the final token, NameFrom returns null instead of reporting the missing value, so a command such as kcap agent stop --all -y --daemon silently resolves to the profile's default daemon and stops all of its agents without confirmation. Validate that --daemon has a non-flag value before resolving or issuing any stop.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| target = ""; |
There was a problem hiding this comment.
In the interactive --all flow, the agent list is fetched before the prompt, but after confirmation target = "" tells the daemon to enumerate its current agents again. If an agent is launched from the web UI or a review flow while the user is considering the prompt, that new, undisclosed agent is also stopped even though it was absent from the displayed list and count; send the confirmed IDs or re-list and reconfirm when the set changes.
Useful? React with 👍 / 👎.
Closes agent-command-group defects found by automated review: a valueless --daemon on ls/attach/stop could silently retarget a destructive stop at the default daemon or crash on an uncaught ArgumentException; StopAck always reported success even when the graceful stop failed or the process never exited; and the README omitted the --yes long form already documented in help-agent.txt. - AgentCommand.DaemonNameFrom validates --daemon has a real value before any socket is touched, reporting a clean usage error instead of resolving the default daemon or crashing. - StopAgentCoreAsync now returns whether the stop was actually confirmed (Runtime.HasExited); StopAck's payload is one id\tstatus line per agent so the CLI can report per-agent success/failure and exit 1 on any failure, matching `kcap daemon stop`.
… review StopAgentCoreAsync read Runtime.HasExited immediately after TerminateAsync's SIGKILL, but TerminateAsync's own post-kill waitpid is non-blocking and usually hasn't reaped the child yet — so a hung agent that needed SIGKILL (the main reason anyone runs `kcap agent stop`) was reported as `failed` with no corresponding daemon-log error. Poll briefly (WaitForExitAsync) after TerminateAsync before treating the stop as unconfirmed. Also closes the same "--daemon/--name with an empty value" validation gap in two more places: AgentCommand.DaemonNameFrom (ls/attach/stop) and AgentStartArgs (agent start), both of which let `--daemon ""` or `--daemon --flag` through to crash instead of printing a clean error. Minor: collapse a redundant double-empty-check in SendStopAsync, and update the FrameCodecTests name/payload, design doc, and README to match the id\tstatus StopAck shape and the failure-reporting behavior it enables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Triaged the bot reviews. Pushed FixedQodo #3 / Codex P1 — valueless Qodo #4 — failed stops reported success. Confirmed and fixed. Worth flagging: the first attempt at this fix was itself wrong, and review caught it. Reading Qodo #1 — DeferredQodo #5 / Codex P2 — Not a defectQodo #2 — "Getting started remains unsynchronized". The README’s |
…ombstone (#392) * Restore the `kcap agent` command group shadowed by the retired-verb tombstone #390 added a `command is "agent"` guard at the top of Program.cs that exits 2 with a pointer to `kcap daemon`. #383 added the `kcap agent start|ls|stop|attach` group at `case "agent":` in the switch below it. They merged without conflict — different regions of the file — and the guard wins, so every `kcap agent` subcommand on main answers "Unknown command: agent" and exits 2. Neither side's tests caught it: #390's assert the tombstone fires (it does), and #383's exercise the parsers and daemon handlers directly, never through Program.cs. AgentVerbDispatchTests now spawns the real binary and pins that the group is reachable, which is the gap that let this through. The daemon signpost is kept where it doesn't collide: `kcap agent status`, `restart`, `logs`, `doctor`, and `service` only ever meant the daemon, so they answer with a pointer to `kcap daemon <verb>`. `start` and `stop` belong to both groups and dispatch to the agent group. Note this is reachable only with a server configured — the `agent` group resolves config before dispatch, unlike the pre-config tombstone it replaces. Closes #391 AI-1570 Reverts the CLI behaviour of #390 (AI-1569). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove the retired-verb text ban; fix dispatch, Windows and offline gaps Per the decision to keep `kcap agent` as the coding-agent group, drops RetiredVerbScanTests (AI-1567), which banned the literal string "kcap agent" from all shipped text under kcap/, src/ and npm/ and is what failed CI on both platforms. Addresses the review findings on this PR: - The dispatch test did not prove dispatch. In a config-less environment every non-help case exited 1 at the missing-server gate and `--help` returned from the global help path, so nothing pinned that AgentCommand was reached — another pre-dispatch guard would have passed it. Each case now asserts a string only AgentCommand emits, and points `ls` at a daemon name that cannot exist so the handler's own output is deterministic. - Windows bypassed the daemon signpost. NotSupportedOnWindows returned before the subcommand switch, so `kcap agent status` reported that coding agents are unsupported instead of pointing at `kcap daemon status`, which Windows does support. The signpost now runs ahead of the platform guard. - The group sat behind the server-config gate, which made the signpost unreachable offline. `agent` joins offlineCommands and `start` — the only subcommand that needs a server for the daemon to record to — reports the missing server itself. ls/attach/stop only ever talk to the local socket. Two defects the rewritten test then caught in the group itself: - Any argv[1] was treated as a subcommand, so `kcap agent --daemon dev` failed with "unknown subcommand '--daemon'" instead of listing that daemon's agents. A leading flag is now an `ls` option. - `--no-update-check` is global and left in argv by Program.cs, so strict parsing read it as a subcommand and, worse, as a vendor name for `start`. Global flags are dropped before the split. Closes #391 AI-1570 Reverts the CLI behaviour of #390 (AI-1569) and the text ban from #389 (AI-1567). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 participant
Closes #382
AI-1555
Regroups the three top-level agent commands into one
kcap agentgroup, and adds astopsubcommand that did not exist before.Command surface
Breaking change
kcap run-agent,kcap attach, andkcap lsare removed, not aliased. Two flags are respelled:--name→--daemon(it always meant the daemon name, which reads as an agent name once there is astopsubcommand) and--detached→-d/--detach(matchingkcap daemon start -d). The old spellings are rejected as unknown flags rather than silently accepted.What else changed
kcap agent stopis new end to end: two append-only local-IPC frames (Stop = 8,StopAck = 70), aHandleLocalStopAsyncon the daemon, and the CLI subcommand. It reuses the existing graceful path —/exit, 15s wait, then terminate.--privateagents, which nothing could stop before.HandleStopAgent’s body moved intoStopAgentCoreAsync; theIsPrivateguard stays on the server-origin path (defence-in-depth against a leaked id) and only the 0600 local socket bypasses it. The core skips the two_server.*calls for private agents, since an unregistered agent has no server-side row.attachandstopaccept any unique prefix; an ambiguous one lists the candidates. A full 32-hex id is sent verbatim so the daemon’s PID-record fallback can still reap survivors of a prior daemon incarnation.kcap --helpnow lists these commands at all — previously all three were absent fromhelp-usage.txt.Verification
CodexHookCommandTestsand the uninstall/config.tomlarea — the merge base7a4975afails the identical 42. Zero new failures.dotnet publish -c Releaseclean of IL3050/IL2026.kcap agent start/ls/stop/attachexercised live against a real daemon and the realclaudeCLI, including stop-by-prefix,--allwith cancel/confirm/-y, unknown id, and the too-old-daemon path.Known follow-ups
stop --allstops every agent the daemon hosts, including server-launched agents and review-flow participants; that is now documented inhelp-agent.txtand the README, but the real fix is kcap agent commands should be aware of flow participants #379.AgentListresponse classifier and a stop-arg validator asinternal staticseams (followingResolveAgentId’s precedent) so two fixed defects get regression pins.Design and plan
docs/superpowers/specs/2026-07-28-ai1555-agent-command-group-design.mddocs/superpowers/plans/2026-07-28-ai1555-agent-command-group.md🤖 Generated with Claude Code