Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -435,17 +435,29 @@ def resolve_notify_agent(topic, requested_task_id)
# When task_id is absent, fall back to topic.primary_agent for
# back-compat with older plugin builds that don't echo task_id.
#
# If task_id IS supplied but the delegated task lookup fails (late
# reply after cancel/timeout, stale id, wrong topic), do NOT fall
# through to primary_agent — a present-but-unresolved task_id means
# the client believes it is answering a specific dispatch that no
# longer exists. Saving the reply against primary_agent would create
# an unlinked comment while complete_delegated_task finds nothing,
# potentially duplicating a reply after the task was already
# completed/cancelled. Return nil so reply renders 403.
# If task_id IS supplied but no task with that id exists on this topic
# (stale id, wrong topic) — or it belongs to a different/non-Claude
# agent — do NOT fall through to primary_agent. A present-but-foreign
# task_id means the client believes it is answering a dispatch it does
# not actually own; saving against primary_agent would create an
# unlinked comment. Return nil → reply renders 403.
#
# BUT do NOT gate this lookup on status == "delegated". Multiple Claude
# Channel sessions sharing one agent (the default AGENT_NAME case: two
# `claude --channels` sessions in the same working directory) all stream
# from agent:user:<id>, so a single work-topic dispatch fans out to every
# session and each calls /reply with the same task_id. The intended dedup
# is the atomic claim in claim_delegated_task (WHERE status='delegated'),
# which yields a clean 409 "already completed" for the loser — a signal
# the MCP plugin treats as benign. Filtering on status HERE made the loser
# fail one layer too early: the sibling already flipped the task to "done",
# this lookup returned nil, and reply rendered a misleading 403 "Not
# authorized" that the plugin surfaced as a hard error. Resolve the agent
# for any task on THIS topic owned by the caller's Claude Channel agent,
# regardless of status, and let claim_delegated_task decide actionability.
def resolve_reply_agent(topic, requested_task_id)
if requested_task_id.present?
task = Task.where(topic_id: topic.id, status: "delegated").find_by(id: requested_task_id)
task = Task.where(topic_id: topic.id).find_by(id: requested_task_id)
agent = task&.agent
return nil unless agent && agent.claude_channel_agent? && agent.created_by_id == current_user.id

Expand Down
18 changes: 10 additions & 8 deletions engines/collavre/test/controllers/api/v1/agents_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -998,18 +998,20 @@ class AgentsControllerTest < ActionDispatch::IntegrationTest
assert_equal "done", task.reload.status

# Second reply (same task_id) arrives after the first has completed.
# The task is no longer in delegated state — resolve_reply_agent
# cannot find it, returns nil, and reply renders 403. Either way,
# no duplicate comment is created.
# This is the multi-session dedup case: two Claude Channel sessions
# sharing one agent both /reply to the same fanned-out dispatch. The
# loser must reach the atomic-claim layer and get a clean 409 "already
# completed" — NOT a 403. resolve_reply_agent no longer gates on
# status: "delegated", so the already-done task still resolves the
# agent, and claim_delegated_task returns the 409. (Before the fix the
# loser failed at resolve_reply_agent with a misleading 403 that the
# MCP plugin surfaced as a hard error.) No duplicate comment either way.
post "/api/v1/agent/reply",
params: { topic_id: topic_id, text: "Duplicate reply", task_id: task.id },
headers: auth_headers,
as: :json
# The post-completion lookup falls through resolve_reply_agent (which
# scopes to status: "delegated") → 403. Either way, the second
# request must not produce a 2xx.
refute_includes 200..299, response.status,
"second reply for an already-completed task must not return 2xx"
assert_response :conflict,
"loser of the shared-agent dispatch race must get 409 (benign dedup), not 403"

# Count only the agent's own reply comments. An agent reply in an inbox
# creative now also creates a system-authored (user: nil) notification in
Expand Down
17 changes: 15 additions & 2 deletions tools/collavre-claude-plugin/src/collavre-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class CollavreClient {
topicId: number,
text: string,
taskId: number,
): Promise<{ comment_id: number }> {
): Promise<{ handled: true; comment_id: number } | { handled: false; reason: string }> {
const body: Record<string, unknown> = { topic_id: topicId, text, task_id: taskId };

const res = await fetch(`${this.baseUrl}/api/v1/agent/reply`, {
Expand All @@ -85,12 +85,25 @@ export class CollavreClient {
body: JSON.stringify(body),
});

// 409 Conflict is the server's benign dedup signal, not an error. Multiple
// Claude Code sessions in the SAME working directory share one Collavre
// agent; a work-topic dispatch fans out to all of them and each session's
// turn calls reply with the same task_id. The server's atomic task claim
// lets exactly one win — the others get 409 "already completed". The message
// was already delivered by a sibling, so this is success-with-nothing-to-do,
// not a failure to surface to the model.
if (res.status === 409) {
const respBody = await res.text();
return { handled: false, reason: respBody };
Comment on lines +95 to +97

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not swallow every reply conflict as dedup

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 👍 / 👎.

}

if (!res.ok) {
const respBody = await res.text();
throw new Error(`Reply failed (${res.status}): ${respBody}`);
}

return res.json() as Promise<{ comment_id: number }>;
const json = (await res.json()) as { comment_id: number };
return { handled: true, comment_id: json.comment_id };
}

// Post an out-of-band informational comment to a topic WITHOUT completing a
Expand Down
18 changes: 17 additions & 1 deletion tools/collavre-claude-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ function buildServer(

const result = await client.reply(topicId, text, taskId);

// The dispatched turn is concluding (Claude has replied). Reset the active
// The dispatched turn is concluding (Claude has replied, or a sibling
// session already answered this fanned-out dispatch). Reset the active
// context to the registration inbox default so a subsequent locally-
// initiated turn's permission prompt surfaces in the inbox rather than
// leaking into this just-finished work topic.
Expand All @@ -140,6 +141,21 @@ function buildServer(
// comment must not be claimed and forwarded to a turn that is already over.
coordinator.clear();

// Benign dedup: another session sharing this agent already claimed and
// answered this dispatch (server returned 409). Nothing was posted by us,
// and that is correct — report it as a non-error so the model doesn't treat
// a normal multi-session race as a failed reply.
if (!result.handled) {
return {
content: [
{
type: "text" as const,
text: "Already answered by another session sharing this agent — nothing to send.",
},
],
};
}

return {
content: [
{ type: "text" as const, text: `Sent (comment #${result.comment_id})` },
Expand Down
Loading