Repository navigation
Report each hosted agent's slash commands for the composer picker - #1031
Conversation
Commands are unknown at registration, so they ride a new best-effort ReportAgentCommands hub method rather than AgentRegistered: an ACP available_commands_update, a Codex skills/list, or a headless Claude initialize probe (KCAP_SKIP=1 keeps the probe from recording a session). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR Summary by QodoReport hosted-agent slash commands to the composer picker
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| IReadOnlyList<HostedAgentCommand>? pending; | ||
| lock (_lock) { | ||
| _callback = value; | ||
| pending = _last; | ||
| } | ||
|
|
||
| if (value is not null && pending is not null) value(pending); | ||
| } |
There was a problem hiding this comment.
10. Newer command updates can be overwritten 🐞 Bug ≡ Correctness
HostedAgentCommandsRelay.Callback captures the pending _last list and callback under its lock but invokes the callback after releasing the lock. If Publish replaces _last and delivers a newer update before that delayed invocation, the older handshake snapshot arrives last and unconditionally replaces AgentInstance.Commands while also starting a stale server report.
Agent Prompt
## Issue description
The relay can deliver callback snapshots in reverse order because callback attachment captures a buffered value under the state lock but invokes it outside the lock without a version check. A concurrent publication can therefore deliver a newer command list before the older attach-time handshake snapshot, allowing the older list to overwrite local and server state.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/HostedAgentCommandsRelay.cs[14-34]
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/HostedAgentCommandsRelayTests.cs[12-43]
## Recommended Fix
Associate each published list with a monotonically increasing version and coordinate callback attachment and publication so delivery order follows publication order without executing arbitrary user callbacks while holding the relay lock. Deliver a buffered attach-time snapshot only if it is still current, and add a deterministic concurrency test that pauses attachment after it captures the buffered snapshot, publishes a newer list, and verifies that the older list can never be delivered last.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The server drops a command report for an agent it has not registered yet, so attach the callback (which flushes the handshake snapshot) and probe Claude only after AgentRegistered is acked. Resend the cached list on re-registration too — a same-slot re-register replaces the server entry with a null-Commands record, and a server restart wiped it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Claude probe moves into the daemon assembly so it stops its child through ProcessTree.Kill, and it drains stderr so a full stderr pipe can't deadlock it. Codex skills/list runs off the launch-critical path so a slow request can't delay the first turn. The relay versions each list so a late attach-time snapshot can't overwrite a newer one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Qodo disposition (the "Code Review by Qodo" summary above predates the fix commits): all 10 inline findings are addressed. Fixed in later commits — #1 skills/list off the launch-critical path, #2 reconnect replay, #4 Codex comment reworded, #5 relay test comments trimmed, #6 cancelled-launch (resolved by #1), #7 stderr drain, #8 |
1 participant
Closes #747 — AI-2428
What & why
The daemon now reports each hosted agent's slash commands to the server for the composer's
/picker. Commands are unknown at registration, so they ride a new best-effortReportAgentCommandshub method rather thanAgentRegistered: ACP vendors forward theavailable_commands_updatethey already receive (plus thesession/newresult), Codex callsskills/list, and Claude is probed headlessly with-p --input-format stream-jsonand aninitializecontrol request.Where to look
ClaudeCommandProbeis net new —KCAP_SKIP=1keeps its SessionStart hooks from recording a phantom session, and an empty strict MCP config keeps it from spawning the worktree's MCP servers while still surfacing built-in and plugin commands. A buffered relay hands the orchestrator a list captured during the handshake the instant its callback attaches, so nothing races the attach. Codexskills/list's wire shape is not yet probe-confirmed, so its parser is defensive and fails safe to an empty list.Verification
Daemon builds clean; AOT publish clean (no IL2026/IL3050); 12 new unit tests pass — probe parse against the real 2.1.273 initialize response, ACP/Codex extraction, and the relay's flush-on-attach timing.
🤖 Generated with Claude Code