fix(claude-channel): benign dedup for shared-agent reply race#1309
fix(claude-channel): benign dedup for shared-agent reply race#1309sh1nj1 wants to merge 1 commit into
Conversation
Two Claude Code channel sessions in the same working directory share one Collavre agent (default AGENT_NAME). Both stream from agent:user:<id>, so a single work-topic dispatch fans out to both and each session's turn calls /reply with the same task_id. The server is designed to dedup these via the atomic task claim (claim_delegated_task), returning 409 to the loser. But resolve_reply_agent scoped its task lookup to status: "delegated". Once the winning session flipped the task to "done", the loser's lookup returned nil and reply rendered a misleading 403 "Not authorized" — which the MCP plugin threw as a hard error and surfaced to the user. Server: resolve_reply_agent no longer gates on status. It resolves the agent for any task on the topic owned by the caller's Claude Channel agent and lets claim_delegated_task decide actionability — so the loser reaches the intended 409. Nonexistent/foreign task_ids still 403 (ownership + topic scope unchanged). Plugin: reply() treats 409 as a benign dedup (handled:false) instead of throwing; the reply tool reports "already answered by another session" rather than a failed reply.
![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: 90efe44df7
ℹ️ 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".
| if (res.status === 409) { | ||
| const respBody = await res.text(); | ||
| return { handled: false, reason: respBody }; |
There was a problem hiding this comment.
When the original dispatch is cancelled or failed instead of answered (for example, offline-session cancellation or stuck-task recovery flips a delegated task while Claude is still composing), /reply now also produces a 409 because the server resolves non-delegated tasks and the atomic claim fails. This branch treats every 409 as a benign sibling-session dedup, and index.ts then reports “Already answered” without posting any comment, so a valid response can be silently dropped. Please distinguish the completed/dedup case from cancelled/failed/not-delegated conflicts before suppressing the error.
Useful? React with 👍 / 👎.
1 participant
Problem
Running two
claude --channels plugin:collavre@collavresessions in the same working directory and sending a message in a Collavre topic: both sessions receive the dispatch, the first replies fine, the second fails with:Root cause
Same-directory sessions share one Collavre agent (the default
AGENT_NAMEcase). Both subscribe toagent:user:<id>, so a single work-topic dispatch fans out to both and each session's turn calls/replywith the sametask_id. This is by design —ClaudeChannelAdapterdocuments that a work topic "may be handled by any session (the server's atomic task claim dedups concurrent handlers)", with the loser getting a clean 409.But
resolve_reply_agentscoped its task lookup tostatus: "delegated". Once the winning session'sclaim_delegated_taskflipped the task todone, the loser's lookup returnednilandreplyrendered a misleading 403 at the auth layer — one step before the intended 409 dedup. The MCP plugin throws on any non-2xx, so the 403 surfaced to the user as a hard error.The existing test
concurrent replies for same task_id only one wins; loser gets 409even admitted this in its assertion (weakened torefute 2xxwith a comment noting the real result was 403).Fix
Server (
resolve_reply_agent): drop thestatus: "delegated"filter. Resolve the agent for any task on the topic owned by the caller's Claude Channel agent, and letclaim_delegated_task(which keeps the atomicWHERE status='delegated') decide actionability. The loser now reaches the designed 409. Nonexistent / foreign-agent / wrong-topictask_ids still 403 (ownership + topic scoping unchanged).Plugin (
collavre-client.ts+index.ts):reply()treats 409 as a benign dedup (handled: false) instead of throwing; the reply tool reports "already answered by another session" rather than a failed reply.Verification
engines/collavre/test/controllers/api/v1/agents_controller_test.rb— 69 runs, 0 failures (updated the concurrent-reply test to assert:conflict).tscclean build +npm test(28 pass).🤖 Generated with Claude Code