From 44a4573e6b30a8eedba145052bebe1b22686f4cc Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 04:22:44 +0000 Subject: [PATCH 1/8] deterministic delegation receipts: route a delegated task's outcome back to the delegator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-to-agent delegation had no completion signal: a sub-agent's report was routed by the LLM router, which prefers "stop" and silently swallowed it, so users had to relay results between agents by hand. Backend (workspace_mod): - strip client-supplied server-owned metadata (delegated_by/delegated_to/ receipt_from) so a forged delegation chain can never enter routing - mark explicit @mention delegations from agents with delegated_by/to - deterministically route structured receipts (in_reply_to + reply_kind) back to the delegator, bypassing the LLM router; validated for same workspace/channel, source consistency, membership, TTL (DELEGATION_RECEIPT_TTL_HOURS, default 24h), single terminal receipt per (E1, replier) stamped onto E1 as receipt_from - mentioning the original delegator stays a plain receipt; mentioning a third agent dual-routes [delegator, C] and marks the onward delegation; master mode keeps its star topology (onward mentions ignored) Connector: - BaseAdapter gains sendFinalResult/sendFinalError/sendNeedsInput/ sendCancelled, stamping reply_kind + in_reply_to; synthetic triggers (system:*, no event id) degrade to plain replies - _inflightTurns registry (strict lifecycle) supplies the trigger for control-action cancellations only; Claude's late trailing output stays unstamped rather than guessing attribution - all adapters migrated at their terminal send points; result vs error vs needs_input vs cancelled mapped per path Launcher: reply_kind badge on agent messages (returned / failed / needs input / cancelled) — deliberately not "task completed". Known limits (phase 2): no durable receipt inbox across daemon restarts (_skipExistingEvents jumps to head); Python SDK prompt mirror not updated. --- .../agent-connector/src/adapters/aider.js | 8 +- packages/agent-connector/src/adapters/amp.js | 4 +- packages/agent-connector/src/adapters/base.js | 74 +++- .../agent-connector/src/adapters/claude.js | 26 +- .../agent-connector/src/adapters/cline.js | 16 +- .../agent-connector/src/adapters/codex.js | 26 +- .../agent-connector/src/adapters/copilot.js | 8 +- .../agent-connector/src/adapters/cursor.js | 8 +- .../agent-connector/src/adapters/gemini.js | 4 +- .../agent-connector/src/adapters/goose.js | 5 +- .../agent-connector/src/adapters/hermes.js | 6 +- .../src/adapters/llm-direct.js | 6 +- packages/agent-connector/src/adapters/mini.js | 8 +- .../agent-connector/src/adapters/openclaw.js | 6 +- .../agent-connector/src/adapters/opencode.js | 14 +- .../src/adapters/workspace-prompt.js | 15 +- .../test/claude-decision-pinning.test.js | 3 + packages/agent-connector/test/cline.test.js | 4 + packages/agent-connector/test/codex.test.js | 2 +- packages/agent-connector/test/copilot.test.js | 2 + packages/agent-connector/test/goose.test.js | 2 + packages/agent-connector/test/mini.test.js | 2 + .../agent-connector/test/stop-control.test.js | 15 + .../test/terminal-replies.test.js | 118 ++++++ .../components/chat/MessageBubble.tsx | 20 + .../src/renderer/i18n/locales/en/chat.json | 8 +- .../src/renderer/i18n/locales/zh/chat.json | 8 +- workspace/backend/app/config.py | 6 + workspace/backend/app/mods/workspace_mod.py | 162 +++++++- .../backend/tests/test_receipt_routing.py | 370 ++++++++++++++++++ 30 files changed, 878 insertions(+), 78 deletions(-) create mode 100644 packages/agent-connector/test/terminal-replies.test.js create mode 100644 workspace/backend/tests/test_receipt_routing.py diff --git a/packages/agent-connector/src/adapters/aider.js b/packages/agent-connector/src/adapters/aider.js index 87339c8e6..acbd2a0fc 100644 --- a/packages/agent-connector/src/adapters/aider.js +++ b/packages/agent-connector/src/adapters/aider.js @@ -482,11 +482,13 @@ class AiderAdapter extends BaseAdapter { } const { text, error } = result; if (error) { - await this.sendError(msgChannel, error); + await this.sendFinalError(msg, msgChannel, error); } else if (text) { - await this.sendResponse(msgChannel, text); + await this.sendFinalResult(msg, msgChannel, text); } else { - await this.sendResponse( + // No text but the run succeeded (file edits were applied) — a result. + await this.sendFinalResult( + msg, msgChannel, 'Aider finished with no textual output (any file changes were applied to the working directory).', ); diff --git a/packages/agent-connector/src/adapters/amp.js b/packages/agent-connector/src/adapters/amp.js index b24d3de1c..6ce6b7664 100644 --- a/packages/agent-connector/src/adapters/amp.js +++ b/packages/agent-connector/src/adapters/amp.js @@ -288,9 +288,9 @@ class AmpAdapter extends BaseAdapter { // spawn/runtime error the agent row may be showing. this._reportStatus(null); if (responseText) { - await this.sendResponse(msgChannel, responseText); + await this.sendFinalResult(msg, msgChannel, responseText); } else { - await this.sendResponse(msgChannel, 'No response generated. Please try again.'); + await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } } diff --git a/packages/agent-connector/src/adapters/base.js b/packages/agent-connector/src/adapters/base.js index 8d931a3bb..1fcb867ec 100644 --- a/packages/agent-connector/src/adapters/base.js +++ b/packages/agent-connector/src/adapters/base.js @@ -80,6 +80,12 @@ class BaseAdapter { // Per-channel task tracking for parallel execution this._channelBusy = new Set(); this._channelQueues = {}; + // Inbound message currently being handled per channel. Strict lifecycle: + // set right before _handleMessage, cleared right after. Consumed ONLY by + // cancellation paths (control "stop" has no trigger of its own) — never + // use it to attribute late/background output, which can arrive after the + // next message has already replaced the entry. + this._inflightTurns = {}; // Cached workspace.browser_enabled. Populated lazily on first read so we // don't pay an HTTP roundtrip per message — adapters that toggle the // workspace flag must reconnect/restart to pick up the change (matches @@ -746,11 +752,12 @@ class BaseAdapter { async _channelWorker(channel, msg) { this._channelBusy.add(channel); + this._inflightTurns[channel] = msg; try { await this._handleMessage(msg); } catch (e) { this._log(`Error in channel worker for ${channel}: ${e.message}`); - try { await this.sendError(channel, `Agent error: ${e.message}`); } catch {} + try { await this.sendFinalError(msg, channel, `Agent error: ${e.message}`); } catch {} } // Drain queue @@ -761,13 +768,15 @@ class BaseAdapter { if (nextMsg._queueId) { try { await this.sendStatus(channel, 'processing queued message', { queue_id: nextMsg._queueId, queue_status: 'processed' }); } catch {} } + this._inflightTurns[channel] = nextMsg; try { await this._handleMessage(nextMsg); } catch (e) { this._log(`Error processing queued message in ${channel}: ${e.message}`); - try { await this.sendError(channel, `Agent error: ${e.message}`); } catch {} + try { await this.sendFinalError(nextMsg, channel, `Agent error: ${e.message}`); } catch {} } } + delete this._inflightTurns[channel]; this._channelBusy.delete(channel); } @@ -844,6 +853,67 @@ class BaseAdapter { } } + // ------------------------------------------------------------------ + // Terminal replies (delegation receipts) + // ------------------------------------------------------------------ + // + // Unlike sendResponse, these stamp the outgoing chat message with + // reply_kind + in_reply_to so the backend can deterministically route a + // delegated task's outcome back to the delegating agent instead of + // relying on the LLM router to guess "this is a report". `triggerMsg` is + // the inbound message that started the turn. Synthetic triggers + // (system:* senders, missing event id) degrade to a plain reply — the + // backend only honours receipts that reference a real delegation event, + // so over-stamping is harmless but under-stamping loses the receipt. + + _receiptMeta(kind, triggerMsg) { + const meta = { reply_kind: kind }; + const id = triggerMsg && triggerMsg.messageId; + const sender = (triggerMsg && triggerMsg.senderName) || ''; + if (id && !sender.startsWith('system:')) meta.in_reply_to = id; + return meta; + } + + async _sendTerminal(kind, triggerMsg, channel, content) { + try { + await this.client.sendMessage(this.workspaceId, channel, this.token, content, { + senderType: 'agent', + senderName: this.agentName, + metadata: this._receiptMeta(kind, triggerMsg), + sessionId: this._sessionId, + }); + } catch (e) { + if (e instanceof SessionRevokedError) { + this._onSessionRevoked(); + return; + } + // Result delivery failures must surface to the caller (matching + // sendResponse); error/cancel/needs-input paths swallow like sendError + // so a failing receipt can't mask the original problem. + if (kind === 'result') throw e; + } + } + + /** The turn's final answer. */ + async sendFinalResult(triggerMsg, channel, content) { + return this._sendTerminal('result', triggerMsg, channel, content); + } + + /** The turn failed — the delegator must not wait forever. */ + async sendFinalError(triggerMsg, channel, content) { + return this._sendTerminal('error', triggerMsg, channel, content); + } + + /** The agent needs more input before it can continue. */ + async sendNeedsInput(triggerMsg, channel, content) { + return this._sendTerminal('needs_input', triggerMsg, channel, content); + } + + /** The turn was cancelled (user stop). */ + async sendCancelled(triggerMsg, channel, content) { + return this._sendTerminal('cancelled', triggerMsg, channel, content); + } + async cleanupTodos(channel) { try { const result = await this.client.getTodos(this.workspaceId, channel, this.token, { diff --git a/packages/agent-connector/src/adapters/claude.js b/packages/agent-connector/src/adapters/claude.js index aecdf0f5f..8455922bd 100644 --- a/packages/agent-connector/src/adapters/claude.js +++ b/packages/agent-connector/src/adapters/claude.js @@ -349,7 +349,11 @@ class ClaudeAdapter extends BaseAdapter { async _postStopNotice(channel) { if (!channel || this._stopNoticeSent.has(channel)) return; this._stopNoticeSent.add(channel); - try { await this.sendResponse(channel, 'Execution stopped by user.'); } catch {} + // The control-action stop has no inbound message of its own; the + // inflight-turn registry supplies the turn being cancelled so a + // delegating agent gets a `cancelled` receipt instead of silence. + const trigger = this._inflightTurns[channel] || null; + try { await this.sendCancelled(trigger, channel, 'Execution stopped by user.'); } catch {} } async _stopAllProcesses(completionMessage = 'Execution stopped.') { @@ -365,7 +369,7 @@ class ClaudeAdapter extends BaseAdapter { delete this._channelProcesses[channel]; delete this._channelQueues[channel]; try { - await this.sendResponse(channel, completionMessage); + await this.sendCancelled(this._inflightTurns[channel] || null, channel, completionMessage); } catch {} } } @@ -1076,18 +1080,18 @@ class ClaudeAdapter extends BaseAdapter { * error, or — when the CLI ended with neither — a session-reset notice so * the UI never hangs on "thinking…". */ - async _postTurnOutcome(pp, msgChannel, finalResponse) { + async _postTurnOutcome(pp, msgChannel, finalResponse, triggerMsg) { if (finalResponse) { - try { await this.sendResponse(msgChannel, finalResponse); } catch {} + try { await this.sendFinalResult(triggerMsg, msgChannel, finalResponse); } catch {} } else if (pp.lastErrorText) { - try { await this.sendError(msgChannel, this._formatClaudeError(pp.lastErrorText)); } catch {} + try { await this.sendFinalError(triggerMsg, msgChannel, this._formatClaudeError(pp.lastErrorText)); } catch {} } else if (!pp.everPostedAnything) { if (this._channelSessions[msgChannel]) { delete this._channelSessions[msgChannel]; try { this._saveSessions(); } catch {} this._log(`Empty-error result — cleared session for ${msgChannel}`); } - try { await this.sendError(msgChannel, 'The agent hit an error and could not respond. The session was reset — please send the message again.'); } catch {} + try { await this.sendFinalError(triggerMsg, msgChannel, 'The agent hit an error and could not respond. The session was reset — please send the message again.'); } catch {} } } @@ -1196,7 +1200,7 @@ class ClaudeAdapter extends BaseAdapter { // so it rebuilds context from the channel recap. await this._resetSessionForPromptTooLong(msgChannel); } else { - await this._postTurnOutcome(existingPP, msgChannel, finalResponse); + await this._postTurnOutcome(existingPP, msgChannel, finalResponse, msg); this._resetIdleTimer(msgChannel); await this._queueTodoNudge(msgChannel, msg); return; @@ -1331,9 +1335,9 @@ class ClaudeAdapter extends BaseAdapter { } if (!pp.everPostedAnything) { if (pp.lastErrorText) { - try { await this.sendError(msgChannel, this._formatClaudeError(pp.lastErrorText)); } catch {} + try { await this.sendFinalError(msg, msgChannel, this._formatClaudeError(pp.lastErrorText)); } catch {} } else { - try { await this.sendResponse(msgChannel, 'No response generated. Please try again.'); } catch {} + try { await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } catch {} } } break; @@ -1346,13 +1350,13 @@ class ClaudeAdapter extends BaseAdapter { continue; } - await this._postTurnOutcome(pp, msgChannel, finalResponse); + await this._postTurnOutcome(pp, msgChannel, finalResponse, msg); this._resetIdleTimer(msgChannel); await this._queueTodoNudge(msgChannel, msg); break; } catch (e) { this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); break; } } diff --git a/packages/agent-connector/src/adapters/cline.js b/packages/agent-connector/src/adapters/cline.js index 325a38ab2..e7aa455cd 100644 --- a/packages/agent-connector/src/adapters/cline.js +++ b/packages/agent-connector/src/adapters/cline.js @@ -139,7 +139,7 @@ class ClineAdapter extends BaseAdapter { await this._stopProcess(this._channelProcesses[channel]); delete this._channelProcesses[channel]; delete this._channelQueues[channel]; - try { await this.sendResponse(channel, 'Execution stopped by user.'); } catch {} + try { await this.sendCancelled(this._inflightTurns[channel] || null, channel, 'Execution stopped by user.'); } catch {} } else { await this._stopAllProcesses('Execution stopped by user.'); } @@ -187,7 +187,7 @@ class ClineAdapter extends BaseAdapter { await this._stopProcess(proc); delete this._channelProcesses[channel]; delete this._channelQueues[channel]; - try { await this.sendResponse(channel, message); } catch {} + try { await this.sendCancelled(this._inflightTurns[channel] || null, channel, message); } catch {} } } @@ -560,7 +560,7 @@ class ClineAdapter extends BaseAdapter { }); const spawnStartMs = Date.now(); - const result = await this._runCline(channel, clineBin, args, workingDir); + const result = await this._runCline(channel, clineBin, args, workingDir, msg); if (result.userStopped) return; @@ -583,12 +583,12 @@ class ClineAdapter extends BaseAdapter { // Emit final response or a classified error. if (result.finalText) { - try { await this.sendResponse(channel, result.finalText); } catch {} + try { await this.sendFinalResult(msg, channel, result.finalText); } catch {} } else if (result.errorMessage) { const { kind, userMessage } = classifyClineError(result.errorMessage); - try { await this.sendError(channel, this._withAuthHint(userMessage, kind)); } catch {} + try { await this.sendFinalError(msg, channel, this._withAuthHint(userMessage, kind)); } catch {} } else if (!result.anyOutput) { - try { await this.sendResponse(channel, 'No response generated. Please try again.'); } catch {} + try { await this.sendFinalError(msg, channel, 'No response generated. Please try again.'); } catch {} } return; } @@ -626,7 +626,7 @@ class ClineAdapter extends BaseAdapter { * { ok, finalText, errorMessage, anyOutput, userStopped } * `ok` reflects run_result.finishReason === "completed". */ - _runCline(channel, clineBin, args, workingDir) { + _runCline(channel, clineBin, args, workingDir, triggerMsg) { const cleanEnv = { ...(this.agentEnv || process.env) }; const [cmd, ...spawnArgs] = this._spawnableCmd(clineBin, args); @@ -727,7 +727,7 @@ class ClineAdapter extends BaseAdapter { state.anyOutput = true; { const opts = e.options && e.options.length ? `\n\nOptions: ${e.options.join(' · ')}` : ''; - try { await this.sendResponse(channel, `❓ ${e.question || 'The agent is asking for input.'}${opts}`); } catch {} + try { await this.sendNeedsInput(triggerMsg, channel, `❓ ${e.question || 'The agent is asking for input.'}${opts}`); } catch {} } break; case 'notice': diff --git a/packages/agent-connector/src/adapters/codex.js b/packages/agent-connector/src/adapters/codex.js index 22c444df0..0639f2934 100644 --- a/packages/agent-connector/src/adapters/codex.js +++ b/packages/agent-connector/src/adapters/codex.js @@ -264,11 +264,11 @@ class CodexAdapter extends BaseAdapter { await this.sendStatus(msgChannel, 'thinking...'); if (this._useCliMode) { - await this._handleViaSubprocess(content, msgChannel); + await this._handleViaSubprocess(content, msgChannel, msg); } else if (this._directMode) { - await this._handleViaDirectApi(content, msgChannel); + await this._handleViaDirectApi(content, msgChannel, msg); } else { - await this.sendError(msgChannel, 'codex CLI not found. Install with: npm install -g @openai/codex\n\nOr configure OPENAI_API_KEY + OPENAI_BASE_URL for direct API mode.'); + await this.sendFinalError(msg, msgChannel, 'codex CLI not found. Install with: npm install -g @openai/codex\n\nOr configure OPENAI_API_KEY + OPENAI_BASE_URL for direct API mode.'); } } @@ -276,7 +276,7 @@ class CodexAdapter extends BaseAdapter { // CLI subprocess mode (primary) // ------------------------------------------------------------------ - async _handleViaSubprocess(content, msgChannel) { + async _handleViaSubprocess(content, msgChannel, triggerMsg) { const env = { ...(this.agentEnv || process.env) }; // Set model via env if configured @@ -315,7 +315,7 @@ class CodexAdapter extends BaseAdapter { const result = await this._spawnCodex(cmd, env, msgChannel, fullPrompt); if (result.responseText) { - await this.sendResponse(msgChannel, result.responseText); + await this.sendFinalResult(triggerMsg, msgChannel, result.responseText); return; } else if (result.exitCode !== 0 && threadId && attempt === 0) { // Stale thread — clear and retry fresh @@ -327,12 +327,12 @@ class CodexAdapter extends BaseAdapter { // Surface the actual reason (turn.failed message, stderr, exit code) // instead of a generic "no response" — mirrors the OpenCode adapter // so auth/model/network problems are actionable from the chat. - await this._sendRunFailure(msgChannel, result); + await this._sendRunFailure(msgChannel, result, triggerMsg); return; } } catch (e) { this._log(`Error in subprocess: ${e.message}`); - await this.sendError(msgChannel, `⚠️ **Codex couldn't run** — ${CodexAdapter._redact(e.message)}`); + await this.sendFinalError(triggerMsg, msgChannel, `⚠️ **Codex couldn't run** — ${CodexAdapter._redact(e.message)}`); return; } } @@ -470,12 +470,12 @@ class CodexAdapter extends BaseAdapter { * Post a user-visible failure carrying the actual reason a run produced no * reply, instead of a generic "No response generated". */ - async _sendRunFailure(msgChannel, result) { + async _sendRunFailure(msgChannel, result, triggerMsg) { const detail = CodexAdapter._failureDetail(result); const body = detail ? `Codex failed to complete this run.\n\n> ${detail}` : 'Codex finished without producing a reply. Please try again.'; - await this.sendError(msgChannel, `⚠️ **Codex couldn't run** — ${body}`); + await this.sendFinalError(triggerMsg, msgChannel, `⚠️ **Codex couldn't run** — ${body}`); } /** @@ -510,7 +510,7 @@ class CodexAdapter extends BaseAdapter { // Direct HTTP mode (fallback when CLI not available) // ------------------------------------------------------------------ - async _handleViaDirectApi(content, msgChannel) { + async _handleViaDirectApi(content, msgChannel, triggerMsg) { try { const responseText = await this._callCompletionApi(content, msgChannel); if (responseText) { @@ -519,13 +519,13 @@ class CodexAdapter extends BaseAdapter { if (this._conversationHistory.length > MAX_HISTORY_ENTRIES * 2) { this._conversationHistory = this._conversationHistory.slice(-MAX_HISTORY_ENTRIES * 2); } - await this.sendResponse(msgChannel, responseText); + await this.sendFinalResult(triggerMsg, msgChannel, responseText); } else { - await this._sendRunFailure(msgChannel, {}); + await this._sendRunFailure(msgChannel, {}, triggerMsg); } } catch (e) { this._log(`Error in direct API: ${e.message}`); - await this.sendError(msgChannel, `⚠️ **Codex couldn't run** — ${CodexAdapter._redact(e.message)}`); + await this.sendFinalError(triggerMsg, msgChannel, `⚠️ **Codex couldn't run** — ${CodexAdapter._redact(e.message)}`); } } diff --git a/packages/agent-connector/src/adapters/copilot.js b/packages/agent-connector/src/adapters/copilot.js index 29703fa87..f5b1bbfe8 100644 --- a/packages/agent-connector/src/adapters/copilot.js +++ b/packages/agent-connector/src/adapters/copilot.js @@ -469,16 +469,16 @@ class CopilotAdapter extends BaseAdapter { } if (result.errorMessage) { - await this.sendError(channel, result.errorMessage); + await this.sendFinalError(msg, channel, result.errorMessage); return; } const text = (result.finalText || '').trim(); if (text) { - await this.sendResponse(channel, text); + await this.sendFinalResult(msg, channel, text); } else if (result.timedOut) { - await this.sendError(channel, 'Copilot CLI timed out before producing a response.'); + await this.sendFinalError(msg, channel, 'Copilot CLI timed out before producing a response.'); } else { - await this.sendResponse(channel, 'No response generated. Please try again.'); + await this.sendFinalError(msg, channel, 'No response generated. Please try again.'); } return; } diff --git a/packages/agent-connector/src/adapters/cursor.js b/packages/agent-connector/src/adapters/cursor.js index be01d595e..7433e44b3 100644 --- a/packages/agent-connector/src/adapters/cursor.js +++ b/packages/agent-connector/src/adapters/cursor.js @@ -65,7 +65,7 @@ class CursorAdapter extends BaseAdapter { await this._stopProcess(this._channelProcesses[channel]); delete this._channelProcesses[channel]; delete this._channelQueues[channel]; - try { await this.sendResponse(channel, 'Execution stopped.'); } catch {} + try { await this.sendCancelled(this._inflightTurns[channel] || null, channel, 'Execution stopped.'); } catch {} } else { await this._stopAllProcesses('Execution stopped.'); } @@ -163,7 +163,7 @@ class CursorAdapter extends BaseAdapter { await this._stopProcess(proc); delete this._channelProcesses[channel]; delete this._channelQueues[channel]; - try { await this.sendResponse(channel, completionMessage); } catch {} + try { await this.sendCancelled(this._inflightTurns[channel] || null, channel, completionMessage); } catch {} } } @@ -610,7 +610,7 @@ class CursorAdapter extends BaseAdapter { this._saveSessions(); resolve(true); } else if (fullResponse) { - try { await this.sendResponse(msgChannel, fullResponse); } catch {} + try { await this.sendFinalResult(msg, msgChannel, fullResponse); } catch {} resolve(false); } else { resolve(false); @@ -622,7 +622,7 @@ class CursorAdapter extends BaseAdapter { resolve(true); } else { if (!everPostedAnything) { - try { await this.sendResponse(msgChannel, 'No response generated. Please try again.'); } catch {} + try { await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } catch {} } resolve(false); } diff --git a/packages/agent-connector/src/adapters/gemini.js b/packages/agent-connector/src/adapters/gemini.js index 12bfe91a1..d027a49fb 100644 --- a/packages/agent-connector/src/adapters/gemini.js +++ b/packages/agent-connector/src/adapters/gemini.js @@ -427,7 +427,7 @@ class GeminiAdapter extends BaseAdapter { const fullResponse = lastResponseText.join('').trim(); // Gemini deltas are partial strings, no newline needed between them usually, but wait, delta:true means it appends. If it's multiple blocks, we should join with empty string? Let's check `delta: true`. // Actually if delta: true, they are chunks. We pushed them to array. `lastResponseText.join('')` is correct. if (fullResponse) { - try { await this.sendResponse(msgChannel, fullResponse); } catch {} + try { await this.sendFinalResult(msg, msgChannel, fullResponse); } catch {} } resolve(false); } else if (code !== 0 && this._channelSessions[msgChannel]) { @@ -437,7 +437,7 @@ class GeminiAdapter extends BaseAdapter { resolve(true); } else { if (!postedThinking) { - try { await this.sendResponse(msgChannel, 'No response generated. Please try again.'); } catch {} + try { await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } catch {} } resolve(false); } diff --git a/packages/agent-connector/src/adapters/goose.js b/packages/agent-connector/src/adapters/goose.js index dfe3295c6..3b9973d47 100644 --- a/packages/agent-connector/src/adapters/goose.js +++ b/packages/agent-connector/src/adapters/goose.js @@ -367,9 +367,10 @@ class GooseAdapter extends BaseAdapter { const result = await this._runGoose(content, msgChannel); if (result === null) return; if (result) { - await this.sendResponse(msgChannel, result); + await this.sendFinalResult(msg, msgChannel, result); } else { - await this.sendResponse( + await this.sendFinalError( + msg, msgChannel, 'Goose ran but produced no response. This usually means no provider/model is ' + 'configured — set GOOSE_PROVIDER and GOOSE_MODEL (and a key) for this agent, ' diff --git a/packages/agent-connector/src/adapters/hermes.js b/packages/agent-connector/src/adapters/hermes.js index e8f9d7868..ad2588af2 100644 --- a/packages/agent-connector/src/adapters/hermes.js +++ b/packages/agent-connector/src/adapters/hermes.js @@ -416,13 +416,13 @@ class HermesAdapter extends BaseAdapter { const responseText = await this._runHermes(prompt, msgChannel); if (responseText) { - await this.sendResponse(msgChannel, responseText); + await this.sendFinalResult(msg, msgChannel, responseText); } else { - await this.sendResponse(msgChannel, 'No response generated. Please try again.'); + await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } } catch (e) { this._log(`Hermes adapter error: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); } } } diff --git a/packages/agent-connector/src/adapters/llm-direct.js b/packages/agent-connector/src/adapters/llm-direct.js index 9618de3a5..82a2caa14 100644 --- a/packages/agent-connector/src/adapters/llm-direct.js +++ b/packages/agent-connector/src/adapters/llm-direct.js @@ -113,13 +113,13 @@ class LlmDirectAdapter extends BaseAdapter { if (this._conversationHistory.length > MAX_HISTORY * 2) { this._conversationHistory = this._conversationHistory.slice(-MAX_HISTORY * 2); } - await this.sendResponse(msgChannel, responseText); + await this.sendFinalResult(msg, msgChannel, responseText); } else { - await this.sendResponse(msgChannel, 'No response generated. Please try again.'); + await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } } catch (e) { this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); } } diff --git a/packages/agent-connector/src/adapters/mini.js b/packages/agent-connector/src/adapters/mini.js index 4bf467aaa..3b03d2fb7 100644 --- a/packages/agent-connector/src/adapters/mini.js +++ b/packages/agent-connector/src/adapters/mini.js @@ -393,15 +393,17 @@ class MiniSweAgentAdapter extends BaseAdapter { } const { text, error } = result; if (error) { - await this.sendError(msgChannel, error); + await this.sendFinalError(msg, msgChannel, error); return; } // A successful run proves the runtime is healthy — clear any prior error. this._reportStatus(null); if (text) { - await this.sendResponse(msgChannel, text); + await this.sendFinalResult(msg, msgChannel, text); } else { - await this.sendResponse( + // No text but the run succeeded (file edits were applied) — a result. + await this.sendFinalResult( + msg, msgChannel, 'mini-SWE-agent finished with no textual output (any file changes were applied ' + 'to the workspace directory).', diff --git a/packages/agent-connector/src/adapters/openclaw.js b/packages/agent-connector/src/adapters/openclaw.js index 5d2b014df..8d2ac7449 100644 --- a/packages/agent-connector/src/adapters/openclaw.js +++ b/packages/agent-connector/src/adapters/openclaw.js @@ -238,13 +238,13 @@ class OpenClawAdapter extends BaseAdapter { const responseText = await this._runCliAgent(content, msgChannel); if (responseText) { - await this.sendResponse(msgChannel, responseText); + await this.sendFinalResult(msg, msgChannel, responseText); } else { - await this.sendResponse(msgChannel, 'No response generated. Please try again.'); + await this.sendFinalError(msg, msgChannel, 'No response generated. Please try again.'); } } catch (e) { this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); } } diff --git a/packages/agent-connector/src/adapters/opencode.js b/packages/agent-connector/src/adapters/opencode.js index 94c0d65e8..5a683df9d 100644 --- a/packages/agent-connector/src/adapters/opencode.js +++ b/packages/agent-connector/src/adapters/opencode.js @@ -193,7 +193,7 @@ class OpenCodeAdapter extends BaseAdapter { delete this._channelQueues[channel]; if (proc || hadQueuedWork) { try { - await this.sendResponse(channel, 'Execution stopped by user.'); + await this.sendCancelled(this._inflightTurns[channel] || null, channel, 'Execution stopped by user.'); } catch {} } } else { @@ -344,7 +344,7 @@ class OpenCodeAdapter extends BaseAdapter { // is thrown with a category and handled in catch, so we no longer post a // generic "produced no response" that misattributes everything to auth. if (responseText) { - await this.sendResponse(msgChannel, responseText); + await this.sendFinalResult(msg, msgChannel, responseText); } } catch (e) { if (this._stoppingChannels.has(msgChannel)) { @@ -354,7 +354,7 @@ class OpenCodeAdapter extends BaseAdapter { const category = (e && e.category) ? e.category : this._classifyErrno(e); const diagnostic = OpenCodeAdapter._redact((e && (e.diagnostic || e.message)) || ''); this._log(`OpenCode failure [${category}] in ${msgChannel}: ${diagnostic.slice(0, 300)}`); - await this._sendClassifiedError(msgChannel, category, e && e.detail); + await this._sendClassifiedError(msgChannel, category, e && e.detail, msg); } } @@ -642,7 +642,7 @@ class OpenCodeAdapter extends BaseAdapter { delete this._channelProcesses[channel]; delete this._channelQueues[channel]; try { - await this.sendResponse(channel, completionMessage); + await this.sendCancelled(this._inflightTurns[channel] || null, channel, completionMessage); } catch {} } } @@ -976,7 +976,7 @@ class OpenCodeAdapter extends BaseAdapter { * Post a user-visible, de-identified, actionable error that is clearly NOT a * normal reply. Carries `error_category` in metadata so the UI can route it. */ - async _sendClassifiedError(channel, category, detail) { + async _sendClassifiedError(channel, category, detail, triggerMsg) { const base = FAILURE_MESSAGES[category] || FAILURE_MESSAGES.unknown_error; const safe = detail ? OpenCodeAdapter._redact(detail).trim() : ''; const body = safe ? `${base}\n\n> ${safe}` : base; @@ -986,13 +986,13 @@ class OpenCodeAdapter extends BaseAdapter { senderType: 'agent', senderName: this.agentName, messageType: 'error', - metadata: { agent_mode: this._mode, error: true, error_category: category }, + metadata: { agent_mode: this._mode, error: true, error_category: category, ...this._receiptMeta('error', triggerMsg) }, sessionId: this._sessionId, }); } catch { // Older backends may reject an unknown messageType/metadata — fall back to // the plain error path so the user still gets an actionable message. - try { await this.sendError(channel, content); } catch {} + try { await this.sendFinalError(triggerMsg, channel, content); } catch {} } } diff --git a/packages/agent-connector/src/adapters/workspace-prompt.js b/packages/agent-connector/src/adapters/workspace-prompt.js index 1e9ad2d6a..90c0411af 100644 --- a/packages/agent-connector/src/adapters/workspace-prompt.js +++ b/packages/agent-connector/src/adapters/workspace-prompt.js @@ -169,11 +169,20 @@ function buildCollaborationPrompt(toolMode = 'mcp', skillName = 'openagents-work return ( '\n## Multi-Agent Collaboration\n' + 'To delegate work to another agent, @mention them in your response. ' + - 'Only @mentioned agents will receive the message.\n\n' + + 'Only @mentioned agents will receive the message. In dynamic mode a ' + + 'message is routed to ONE agent — to delegate to several agents, send ' + + 'separate delegations (or use master mode). In master mode only the ' + + 'master delegates directly: a sub-agent\'s message always returns to ' + + 'the master first, even if it @mentions another sub-agent.\n\n' + + 'If the task you just finished was delegated to you by another agent, ' + + 'your final reply is automatically delivered back to that agent — ' + + 'write it as a work report (what you did, the results, anything ' + + 'blocking). Do NOT @mention the delegator in that report; an ' + + '@mention is treated as delegating new work.\n\n' + 'IMPORTANT: Do NOT @mention an agent just to say thanks or acknowledge ' + '— that wakes them up for nothing. Only @mention when you need them ' + - 'to do work. When the task is complete, report results to the user ' + - 'without @mentioning other agents.\n\n' + + 'to do work. When a task for the user is complete, report results to ' + + 'the user without @mentioning other agents.\n\n' + discover ); } diff --git a/packages/agent-connector/test/claude-decision-pinning.test.js b/packages/agent-connector/test/claude-decision-pinning.test.js index 8733a9962..ee4e7f39a 100644 --- a/packages/agent-connector/test/claude-decision-pinning.test.js +++ b/packages/agent-connector/test/claude-decision-pinning.test.js @@ -31,6 +31,9 @@ function mkAdapter(overrides = {}) { adapter.sendStatus = async (ch, text) => { adapter.statuses.push(text); }; adapter.sendResponse = async (ch, text) => { adapter.responses.push(text); }; adapter.sendError = async (ch, text) => { adapter.errors.push(text); }; + adapter.sendFinalResult = async (t, ch, text) => { adapter.responses.push(text); }; + adapter.sendFinalError = async (t, ch, text) => { adapter.errors.push(text); }; + adapter.sendCancelled = async (t, ch, text) => { adapter.responses.push(text); }; adapter.sendThinking = async () => {}; adapter.getRemainingTodos = async () => []; adapter.getBrowserEnabled = async () => false; diff --git a/packages/agent-connector/test/cline.test.js b/packages/agent-connector/test/cline.test.js index 484e790e4..53bd977d4 100644 --- a/packages/agent-connector/test/cline.test.js +++ b/packages/agent-connector/test/cline.test.js @@ -108,6 +108,10 @@ function makeAdapter(extra = {}) { a.sendStatus = async (_c, t) => { a._captured.status.push(t); }; a.sendResponse = async (_c, t) => { a._captured.response.push(t); }; a.sendError = async (_c, t) => { a._captured.error.push(t); }; + a.sendFinalResult = async (_t, _c, t) => { a._captured.response.push(t); }; + a.sendFinalError = async (_t, _c, t) => { a._captured.error.push(t); }; + a.sendNeedsInput = async (_t, _c, t) => { a._captured.response.push(t); }; + a.sendCancelled = async (_t, _c, t) => { a._captured.response.push(t); }; a.sendTodos = async (_c, t) => { a._captured.todos.push(t); }; a._log = () => {}; // Stub the workspace client calls _handleMessage makes (all wrapped in diff --git a/packages/agent-connector/test/codex.test.js b/packages/agent-connector/test/codex.test.js index 1bf69004e..5012108c0 100644 --- a/packages/agent-connector/test/codex.test.js +++ b/packages/agent-connector/test/codex.test.js @@ -82,7 +82,7 @@ describe('Codex — failure detail selection', () => { describe('Codex — user-visible failure message', () => { async function capture(result) { const sent = []; - const fake = { sendError: async (channel, content) => { sent.push({ channel, content }); } }; + const fake = { sendFinalError: async (trigger, channel, content) => { sent.push({ channel, content }); } }; await CodexAdapter.prototype._sendRunFailure.call(fake, 'chan', result); return sent[0]; } diff --git a/packages/agent-connector/test/copilot.test.js b/packages/agent-connector/test/copilot.test.js index 3466613c5..f0d6fe4c1 100644 --- a/packages/agent-connector/test/copilot.test.js +++ b/packages/agent-connector/test/copilot.test.js @@ -52,6 +52,8 @@ function makeAdapter({ scenario = 'success', mode = 'execute', workingDir, model a.sendThinking = async (ch, content) => { a.sent.push({ type: 'thinking', ch, content }); }; a.sendResponse = async (ch, content) => { a.sent.push({ type: 'response', ch, content }); }; a.sendError = async (ch, content) => { a.sent.push({ type: 'error', ch, content }); }; + a.sendFinalResult = async (t, ch, content) => { a.sent.push({ type: 'response', ch, content }); }; + a.sendFinalError = async (t, ch, content) => { a.sent.push({ type: 'error', ch, content }); }; // Capture logs to assert redaction. a.logs = []; a._log = (m) => { a.logs.push(m); }; diff --git a/packages/agent-connector/test/goose.test.js b/packages/agent-connector/test/goose.test.js index 658eee624..aeed4a8de 100644 --- a/packages/agent-connector/test/goose.test.js +++ b/packages/agent-connector/test/goose.test.js @@ -417,6 +417,8 @@ function instrument(a) { a.sendThinking = async (_ch, c) => { sent.thinking.push(c); }; a.sendResponse = async (_ch, c) => { sent.response.push(c); }; a.sendError = async (_ch, c) => { sent.error.push(c); }; + a.sendFinalResult = async (_t, _ch, c) => { sent.response.push(c); }; + a.sendFinalError = async (_t, _ch, c) => { sent.error.push(c); }; a._autoTitleChannel = async () => {}; return sent; } diff --git a/packages/agent-connector/test/mini.test.js b/packages/agent-connector/test/mini.test.js index 31ca425ed..eb6c80a59 100644 --- a/packages/agent-connector/test/mini.test.js +++ b/packages/agent-connector/test/mini.test.js @@ -74,6 +74,8 @@ function makeAdapter(extra = {}) { adapter.sendStatus = async (_c, content) => adapter._streamed.status.push(content); adapter.sendResponse = async (_c, content) => adapter._streamed.response.push(content); adapter.sendError = async (_c, content) => adapter._streamed.error.push(content); + adapter.sendFinalResult = async (_t, _c, content) => adapter._streamed.response.push(content); + adapter.sendFinalError = async (_t, _c, content) => adapter._streamed.error.push(content); return adapter; } diff --git a/packages/agent-connector/test/stop-control.test.js b/packages/agent-connector/test/stop-control.test.js index 71926824f..62ca7c7da 100644 --- a/packages/agent-connector/test/stop-control.test.js +++ b/packages/agent-connector/test/stop-control.test.js @@ -76,6 +76,8 @@ describe('agent stop control', () => { adapter._stopProcess = async () => {}; const responses = []; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); await adapter._stopAllProcesses('Execution stopped by user'); @@ -102,6 +104,8 @@ describe('agent stop control', () => { adapter._stopProcess = async () => {}; const responses = []; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); await adapter._onControlAction('stop', { channel: 'channelA' }); @@ -166,6 +170,8 @@ describe('agent stop control', () => { adapter._stopProcess = async () => {}; const responses = []; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); await adapter._stopAllProcesses('Execution stopped by user'); @@ -192,6 +198,8 @@ describe('agent stop control', () => { adapter._stopProcess = async () => {}; const responses = []; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); await adapter._onControlAction('stop', { channel: 'channelA' }); @@ -219,6 +227,8 @@ describe('agent stop control', () => { adapter._stopProcess = async () => { stopCalls++; }; const responses = []; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); await adapter._onControlAction('stop', { channel: 'channelA' }); @@ -244,6 +254,8 @@ describe('agent stop control', () => { adapter._stopProcess = async () => {}; const responses = []; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); adapter.stop(); await sleep(100); @@ -267,7 +279,10 @@ describe('agent stop control', () => { adapter._autoTitleChannel = async () => {}; adapter.sendStatus = async () => {}; adapter.sendResponse = async (channel, content) => responses.push({ channel, content }); + adapter.sendCancelled = async (trigger, channel, content) => responses.push({ channel, content }); + adapter.sendFinalResult = async (trigger, channel, content) => responses.push({ channel, content }); adapter.sendError = async (channel, content) => errors.push({ channel, content }); + adapter.sendFinalError = async (trigger, channel, content) => errors.push({ channel, content }); adapter._runOpencode = async (_content, channel) => { adapter._stoppingChannels.add(channel); return 'late response after stop'; diff --git a/packages/agent-connector/test/terminal-replies.test.js b/packages/agent-connector/test/terminal-replies.test.js new file mode 100644 index 000000000..0600e5bd7 --- /dev/null +++ b/packages/agent-connector/test/terminal-replies.test.js @@ -0,0 +1,118 @@ +'use strict'; + +/** + * Tests for the terminal-reply (delegation receipt) APIs in BaseAdapter: + * - sendFinalResult/Error/NeedsInput/Cancelled stamp reply_kind + in_reply_to + * - synthetic triggers (system:* senders, missing event id) are NOT stamped + * with in_reply_to, so they can never be mistaken for receipts + * - the inflight-turn registry follows the strict set/clear lifecycle across + * the queue drain, and is available to cancellation paths + * - error-kind failures are swallowed, result-kind failures propagate + * (matching sendError/sendResponse semantics) + */ + +const { describe, it } = require('node:test'); +const assert = require('node:assert/strict'); + +const BaseAdapter = require('../src/adapters/base'); + +function mkAdapter(overrides = {}) { + const adapter = new BaseAdapter({ + workspaceId: `test-ws-${Math.random().toString(36).slice(2)}`, + channelName: 'general', + token: 'tok', + agentName: 'tester', + ...overrides, + }); + adapter.sent = []; + adapter.client = { + sendMessage: async (wsId, channel, token, content, opts) => { + adapter.sent.push({ channel, content, opts }); + }, + }; + return adapter; +} + +const TRIGGER = { messageId: 'evt-123', senderType: 'agent', senderName: 'delegator-a', content: 'do X' }; + +describe('terminal reply metadata', () => { + it('stamps reply_kind and in_reply_to for a real trigger', async () => { + const a = mkAdapter(); + await a.sendFinalResult(TRIGGER, 'general', 'all done'); + assert.equal(a.sent.length, 1); + const meta = a.sent[0].opts.metadata; + assert.equal(meta.reply_kind, 'result'); + assert.equal(meta.in_reply_to, 'evt-123'); + }); + + it('covers all four kinds', async () => { + const a = mkAdapter(); + await a.sendFinalResult(TRIGGER, 'general', 'r'); + await a.sendFinalError(TRIGGER, 'general', 'e'); + await a.sendNeedsInput(TRIGGER, 'general', 'q'); + await a.sendCancelled(TRIGGER, 'general', 'c'); + assert.deepEqual( + a.sent.map((s) => s.opts.metadata.reply_kind), + ['result', 'error', 'needs_input', 'cancelled'], + ); + for (const s of a.sent) assert.equal(s.opts.metadata.in_reply_to, 'evt-123'); + }); + + it('omits in_reply_to for synthetic system triggers', async () => { + const a = mkAdapter(); + await a.sendFinalResult( + { messageId: 'evt-9', senderName: 'system:todos', content: 'nudge' }, + 'general', 'done', + ); + const meta = a.sent[0].opts.metadata; + assert.equal(meta.reply_kind, 'result'); + assert.equal(meta.in_reply_to, undefined); + }); + + it('omits in_reply_to when the trigger is missing or has no event id', async () => { + const a = mkAdapter(); + await a.sendCancelled(null, 'general', 'stopped'); + await a.sendFinalError({ senderName: 'someone' }, 'general', 'boom'); + for (const s of a.sent) assert.equal(s.opts.metadata.in_reply_to, undefined); + }); + + it('swallows send failures for error kind but propagates for result kind', async () => { + const a = mkAdapter(); + a.client.sendMessage = async () => { throw new Error('network down'); }; + await a.sendFinalError(TRIGGER, 'general', 'e'); // must not throw + await assert.rejects(() => a.sendFinalResult(TRIGGER, 'general', 'r'), /network down/); + }); +}); + +describe('inflight turn registry', () => { + it('tracks the current message across the queue drain and clears at the end', async () => { + const a = mkAdapter(); + const seen = []; + a._handleMessage = async (msg) => { + seen.push({ msg: msg.messageId, inflight: a._inflightTurns['general'].messageId }); + if (msg.messageId === 'first') { + a._channelQueues['general'] = [ + { messageId: 'second', senderName: 'human-user', sessionId: 'general' }, + ]; + } + }; + await a._channelWorker('general', { messageId: 'first', senderName: 'human-user', sessionId: 'general' }); + // Each turn saw ITS OWN message as the inflight entry. + assert.deepEqual(seen, [ + { msg: 'first', inflight: 'first' }, + { msg: 'second', inflight: 'second' }, + ]); + assert.equal(a._inflightTurns['general'], undefined); + assert.equal(a._channelBusy.has('general'), false); + }); + + it('worker failure posts a terminal error receipt tied to the failing message', async () => { + const a = mkAdapter(); + a._handleMessage = async () => { throw new Error('adapter exploded'); }; + await a._channelWorker('general', TRIGGER); + assert.equal(a.sent.length, 1); + assert.equal(a.sent[0].opts.metadata.reply_kind, 'error'); + assert.equal(a.sent[0].opts.metadata.in_reply_to, 'evt-123'); + assert.match(a.sent[0].content, /adapter exploded/); + }); +}); diff --git a/packages/launcher/src/renderer/components/chat/MessageBubble.tsx b/packages/launcher/src/renderer/components/chat/MessageBubble.tsx index 01bb40021..bcb0b4ee7 100644 --- a/packages/launcher/src/renderer/components/chat/MessageBubble.tsx +++ b/packages/launcher/src/renderer/components/chat/MessageBubble.tsx @@ -4,6 +4,19 @@ import { cn } from '../../lib/utils' import type { ChatMessage } from '../../types' import Markdown from './Markdown' import ToolCallCard from './ToolCallCard' +import type { VariantProps } from 'class-variance-authority' +import { Badge, badgeVariants } from '../ui/badge' + +// Terminal-reply kinds stamped by adapters (delegation receipts). The badge +// deliberately says "returned", never "task completed" — a finished turn is +// not proof the underlying task is done. Padding and type size ride on the +// Badge `size` prop, not on the variant. +const REPLY_KIND_VARIANT: Record['variant']> = { + result: 'success', + error: 'danger', + needs_input: 'warning', + cancelled: 'muted', +} const AGENT_COLORS = [ 'bg-[#6C63FF]', 'bg-[#FF6B6B]', 'bg-[#26A69A]', 'bg-[#FFA726]', @@ -35,6 +48,10 @@ export default function MessageBubble({ const { t } = useTranslation() const isHuman = message.senderType === 'human' const isSystem = message.senderType === 'system' + const replyKind = !isHuman && !isSystem + ? (message.metadata as { reply_kind?: string } | undefined)?.reply_kind + : undefined + const replyKindVariant = replyKind ? REPLY_KIND_VARIANT[replyKind] : undefined const initials = (message.senderName || '?').slice(0, 2).toUpperCase() const avatarColor = isHuman ? 'bg-[#888]' : isSystem ? 'bg-[#555]' : colorFor(message.senderName || '') @@ -54,6 +71,9 @@ export default function MessageBubble({ {(message.metadata as { agentType?: string }).agentType} )} + {replyKindVariant && ( + {t(`chat.bubble.replyKind.${replyKind}`)} + )} {formatTime(message.createdAt)} {isPending && {t('chat.bubble.sending')}} diff --git a/packages/launcher/src/renderer/i18n/locales/en/chat.json b/packages/launcher/src/renderer/i18n/locales/en/chat.json index 7528a5898..c4c3baf98 100644 --- a/packages/launcher/src/renderer/i18n/locales/en/chat.json +++ b/packages/launcher/src/renderer/i18n/locales/en/chat.json @@ -67,7 +67,13 @@ }, "bubble": { "unknownSender": "unknown", - "sending": "sending…" + "sending": "sending…", + "replyKind": { + "result": "returned", + "error": "failed", + "needs_input": "needs input", + "cancelled": "cancelled" + } }, "toolCall": { "running": "Running…", diff --git a/packages/launcher/src/renderer/i18n/locales/zh/chat.json b/packages/launcher/src/renderer/i18n/locales/zh/chat.json index 446350663..f15bbe018 100644 --- a/packages/launcher/src/renderer/i18n/locales/zh/chat.json +++ b/packages/launcher/src/renderer/i18n/locales/zh/chat.json @@ -67,7 +67,13 @@ }, "bubble": { "unknownSender": "未知", - "sending": "发送中…" + "sending": "发送中…", + "replyKind": { + "result": "已返回", + "error": "执行失败", + "needs_input": "需要输入", + "cancelled": "已取消" + } }, "toolCall": { "running": "运行中…", diff --git a/workspace/backend/app/config.py b/workspace/backend/app/config.py index f6b135a71..a3461fc1b 100644 --- a/workspace/backend/app/config.py +++ b/workspace/backend/app/config.py @@ -78,6 +78,12 @@ class Config: CLOUD_AGENT_MAX_CONTEXT_MESSAGES: int = int(os.environ.get("CLOUD_AGENT_MAX_CONTEXT_MESSAGES", "10")) CLOUD_AGENT_MAX_DEPTH: int = int(os.environ.get("CLOUD_AGENT_MAX_DEPTH", "3")) + # Delegation receipts — how long a delegation message can be referenced by + # a structured receipt (in_reply_to + reply_kind) before it goes stale. + # Product policy, not a technical constant: raise it per deployment when + # agents run tasks longer than a day. + DELEGATION_RECEIPT_TTL_HOURS: float = float(os.environ.get("DELEGATION_RECEIPT_TTL_HOURS", "24")) + # Google OAuth (for "Sign in with Google" Gemini integration) GOOGLE_OAUTH_CLIENT_ID: str = os.environ.get("GOOGLE_OAUTH_CLIENT_ID", "") GOOGLE_OAUTH_CLIENT_SECRET: str = os.environ.get("GOOGLE_OAUTH_CLIENT_SECRET", "") diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index dc1de3d41..efe5d0f2b 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -15,8 +15,9 @@ import logging import re +import time from datetime import datetime, timezone -from typing import List, Optional +from typing import List, Optional, Tuple from sqlalchemy import select @@ -638,6 +639,119 @@ def _master_targets(event, channel, mentions: List[str]) -> List[str]: return [master] +# --------------------------------------------------------------------------- +# Delegation receipts — deterministic "result returns to the delegator" +# --------------------------------------------------------------------------- + +# reply_kind values a connector may stamp on a terminal reply. Anything else +# (missing, empty, unknown) means the message is NOT a structured receipt and +# must fall through to normal routing. +_RECEIPT_REPLY_KINDS = frozenset({"result", "error", "needs_input", "cancelled"}) + +# Metadata keys owned by this mod. Event metadata arrives verbatim from the +# client, so an agent could submit a forged delegation chain; these keys are +# stripped from every inbound message and only written back by the routing +# logic below. +_SERVER_OWNED_METADATA = ("delegated_by", "delegated_to", "receipt_from") + + +def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> Optional[Tuple[List[str], List[str]]]: + """Deterministically route a structured receipt back to its delegator. + + A receipt is an agent chat message stamped (by the connector) with + ``in_reply_to`` — the event id of the delegation message E1 — and a valid + ``reply_kind``. When every check below passes, the reply bypasses the LLM + router and is routed straight to the delegating agent, so a delegated + task's outcome can never be swallowed by a router "stop". + + Returns ``(targets, onward_delegates)`` on a hit — ``targets`` always + starts with the delegator; ``onward_delegates`` are additional agents the + reply @mentions (an onward delegation, dual-routed so the delegator still + sees the outcome). Returns ``None`` when the message is not a valid + receipt, in which case the caller falls through to normal routing. + + Each (E1, replier) pair gets exactly one receipt: on a hit E1 is stamped + with ``receipt_from`` so a later duplicate falls back to normal routing. + If the delegator needs more work done it delegates again, creating a new + E1. + """ + from app.config import config + from app.models import EventRecord + + source = event.source or "" + if not source.startswith("openagents:"): + return None + replier = source[len("openagents:"):] + + meta = event.metadata or {} + if meta.get("reply_kind") not in _RECEIPT_REPLY_KINDS: + return None + ref = meta.get("in_reply_to") + if not ref or not isinstance(ref, str): + return None + + e1 = db.execute( + select(EventRecord).where( + EventRecord.id == ref, + EventRecord.network_id == workspace.id, + ) + ).scalar_one_or_none() + if e1 is None or e1.type != WorkspaceEventTypes.MESSAGE_POSTED: + return None + # Same channel only — a receipt must not redirect traffic across channels. + if e1.target != event.target: + return None + + e1_meta = e1.metadata_ or {} + delegator = e1_meta.get("delegated_by") + # delegated_by is server-written, but still cross-check it against the + # stored source so a routing bug can't be amplified into a misdelivery. + if not delegator or e1.source != f"openagents:{delegator}": + return None + if delegator == replier: + return None + if replier not in (e1_meta.get("delegated_to") or []): + return None + + # Both ends must still be members of this channel — workspace membership + # alone would let a receipt re-target someone who already left the room. + participants = {p.agent_name for p in (channel.participants or [])} + if delegator not in participants or replier not in participants: + return None + + ttl_ms = int(config.DELEGATION_RECEIPT_TTL_HOURS * 3600 * 1000) + if e1.timestamp and (int(time.time() * 1000) - int(e1.timestamp)) > ttl_ms: + return None + + already = e1_meta.get("receipt_from") or [] + if replier in already: + return None + + # Stamp E1 (single terminal receipt per replier). A fresh dict is + # assigned so SQLAlchemy detects the JSONB change. + stamped = dict(e1_meta) + stamped["receipt_from"] = already + [replier] + e1.metadata_ = stamped + db.flush() + + # Onward delegation: mentions other than the original delegator. A "@A + # I'm done" style report must NOT count as a new delegation back to A — + # that would bounce A's acknowledgement to the replier and re-create the + # ping-pong this whole mechanism exists to prevent. In master mode the + # star topology stays authoritative: sub-agents cannot delegate onward, + # so mentions are ignored (matching _master_targets). + mode = (getattr(channel, "orchestration_mode", None) or "dynamic").lower() + onward: List[str] = [] + if mode != "master": + seen = {delegator, replier} + for m in mentions: + if m not in seen and m in participants: + onward.append(m) + seen.add(m) + + return [delegator] + onward, onward + + _ROUTER_PROMPT = """\ You are a conversation router for a multi-agent workspace. Decide which \ agent should respond next to the LATEST message. Use judgment — read the \ @@ -1047,6 +1161,13 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional content = payload.get("content", "") message_type = payload.get("message_type", "chat") + # Event metadata is client-supplied verbatim — drop any inbound values for + # the server-owned delegation keys so a forged delegation chain can never + # enter the routing logic. They are re-written below when this handler + # itself recognises an explicit delegation. + for _key in _SERVER_OWNED_METADATA: + event.metadata.pop(_key, None) + # Reject posts from stale agent sessions. If the sender is an agent # and its claimed session_id does not match the current one in # WorkspaceMember, drop the event and flag it so the router can @@ -1117,7 +1238,26 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional p for p in (channel.participants or []) if p.agent_name != "__no_response__" ] - if len(real_participants) >= 2: + + # ── Structured delegation receipt: deterministic return to delegator ── + # Checked before the participant-count branch so a receipt still lands + # even when the channel has meanwhile shrunk around the pair (the helper + # itself verifies both ends are current participants). + receipt = None + receipt_onward: List[str] = [] + # "error" is included: some adapters post terminal failures with + # message_type="error" (e.g. OpenCode's classified errors), and a failed + # delegation must still return to its delegator. + if message_type in ("chat", "error"): + receipt = _receipt_route(event, channel, mentions, db, workspace) + + if receipt is not None: + targets, receipt_onward = receipt + logger.info( + "workspace_mod: receipt from %s routed to %s in %s", + event.source, targets, channel.name, + ) + elif len(real_participants) >= 2: from app.config import config mode = (getattr(channel, "orchestration_mode", None) or "dynamic").lower() @@ -1145,6 +1285,24 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional else: targets = _fallback_targets(event, channel, mentions, online_names) + # ── Mark explicit delegations (server-owned metadata) ── + # Only agent messages whose routed targets came from the sender's own + # @mentions count as delegations; router-inferred hops (e.g. "report to + # the master") are NOT marked, otherwise the recipient's acknowledgement + # would bounce back as a receipt. Inside a receipt, only the onward + # targets (mentions minus the original delegator) form a new delegation. + if event.source.startswith("openagents:") and targets: + sender = event.source[len("openagents:"):] + if receipt is not None: + delegates = [d for d in receipt_onward if d != sender] + elif mentions and all(t in mentions for t in targets): + delegates = [t for t in targets if t != sender] + else: + delegates = [] + if delegates: + event.metadata["delegated_by"] = sender + event.metadata["delegated_to"] = delegates + # ALWAYS set target_agents, even when nobody should respond. # # Use a non-empty sentinel list ["__no_response__"] instead of [] diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py new file mode 100644 index 000000000..f6ed83abb --- /dev/null +++ b/workspace/backend/tests/test_receipt_routing.py @@ -0,0 +1,370 @@ +# -*- coding: utf-8 -*- +""" +Tests for delegation-receipt routing. + +A structured receipt (agent chat stamped with in_reply_to + reply_kind) must +be routed deterministically back to the delegating agent, bypassing the LLM +router. Everything that is NOT a valid receipt must fall through to the +existing routing untouched. +""" + +import asyncio +import time +import uuid + +import pytest +from unittest.mock import patch, MagicMock + +from app.models import ( + Channel, ChannelMember, EventRecord, Workspace, WorkspaceMember, +) +from app.mods.workspace_mod import _handle_message_posted +from openagents.core.onm_events import Event +from openagents.core.onm_mods import PipelineContext + + +def _run(coro): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + +def _now_ms() -> int: + return int(time.time() * 1000) + + +@pytest.fixture +def ws3(db): + """Workspace with three agents (a, b, c) in one dynamic channel.""" + ws = Workspace(name="Receipt WS", slug=f"receipt-{uuid.uuid4().hex[:8]}", password_hash="t") + db.add(ws) + db.flush() + for name in ("agent-a", "agent-b", "agent-c"): + db.add(WorkspaceMember(workspace_id=ws.id, agent_name=name, role="member", status="online")) + db.flush() + ch = Channel(workspace_id=ws.id, name="session-r", status="active") + db.add(ch) + db.flush() + for name in ("agent-a", "agent-b", "agent-c"): + db.add(ChannelMember(channel_id=ch.id, agent_name=name)) + db.flush() + db.refresh(ch) + return {"workspace": ws, "channel": ch} + + +def _delegation_event(db, ws, *, delegated_by="agent-a", delegated_to=("agent-b",), + target="channel/session-r", timestamp=None, receipt_from=None) -> EventRecord: + """Persist a delegation message E1 the way the pipeline would have.""" + meta = { + "target_agents": list(delegated_to), + "delegated_by": delegated_by, + "delegated_to": list(delegated_to), + } + if receipt_from is not None: + meta["receipt_from"] = list(receipt_from) + rec = EventRecord( + id=str(uuid.uuid4()), + network_id=ws.id, + type="workspace.message.posted", + source=f"openagents:{delegated_by}", + target=target, + payload={"content": f"@{delegated_to[0]} please do X", "message_type": "chat"}, + metadata_=meta, + timestamp=timestamp if timestamp is not None else _now_ms(), + ) + db.add(rec) + db.flush() + return rec + + +def _reply(source, content, *, in_reply_to=None, reply_kind=None, + target="channel/session-r", extra_meta=None) -> Event: + meta = dict(extra_meta or {}) + if in_reply_to is not None: + meta["in_reply_to"] = in_reply_to + if reply_kind is not None: + meta["reply_kind"] = reply_kind + return Event( + type="workspace.message.posted", + source=source, + target=target, + payload={"content": content, "message_type": "chat"}, + metadata=meta, + ) + + +def _ctx(db, ws) -> PipelineContext: + return PipelineContext(network_id=str(ws.id), agent_address="x", db=db, workspace=ws) + + +def _handle(db, ws, event): + return _run(_handle_message_posted(event, _ctx(db, ws))) + + +ROUTER_PATCHES = ( + patch("app.mods.workspace_mod._get_router_api_key", return_value="test-key"), + patch("app.mods.workspace_mod._get_router_model", return_value="claude-haiku-4-5-20251001"), +) + + +def _with_router(mock_text="stop"): + """Context manager stack: mocked LLM router returning `mock_text`.""" + mock_content = MagicMock() + mock_content.text = mock_text + mock_response = MagicMock() + mock_response.content = [mock_content] + mock_client = MagicMock() + mock_client.messages.create.return_value = mock_response + client_patch = patch( + "app.mods.workspace_mod._get_llm_client", + return_value=(mock_client, "anthropic"), + ) + return client_patch, mock_client + + +class TestReceiptHit: + def test_receipt_routes_to_delegator_without_llm(self, db, ws3): + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + client_patch, mock_client = _with_router() + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "All done, results attached.", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a"] + mock_client.messages.create.assert_not_called() + + def test_receipt_stamps_e1_single_use(self, db, ws3): + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + db.refresh(e1) + assert e1.metadata_.get("receipt_from") == ["agent-b"] + + def test_all_reply_kinds_accepted(self, db, ws3): + ws = ws3["workspace"] + for kind in ("result", "error", "needs_input", "cancelled"): + e1 = _delegation_event(db, ws) + out = _handle(db, ws, _reply( + "openagents:agent-b", "terminal", in_reply_to=e1.id, reply_kind=kind, + )) + assert out.metadata["target_agents"] == ["agent-a"], kind + + def test_mention_of_delegator_is_not_reverse_delegation(self, db, ws3): + """"@agent-a I'm done" must stay a plain receipt — no new delegation.""" + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + out = _handle(db, ws, _reply( + "openagents:agent-b", "@agent-a finished, see results.", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a"] + assert "delegated_by" not in out.metadata + assert "delegated_to" not in out.metadata + + def test_onward_mention_dual_routes(self, db, ws3): + """Receipt that @mentions a third agent goes to both A and C, and the + onward hop is marked as a new delegation by B.""" + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + out = _handle(db, ws, _reply( + "openagents:agent-b", "Part one done. @agent-c please take over part two.", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a", "agent-c"] + assert out.metadata["delegated_by"] == "agent-b" + assert out.metadata["delegated_to"] == ["agent-c"] + + +class TestReceiptRejected: + """Every rejection must fall through to normal routing (mocked router).""" + + def _stop_routed(self, db, ws, event): + client_patch, mock_client = _with_router("stop") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, event) + return out, mock_client + + def test_missing_reply_kind(self, db, ws3): + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + out, mock_client = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, + )) + assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_called() + + def test_unknown_reply_kind(self, db, ws3): + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="finished", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_forged_server_owned_metadata_is_stripped(self, db, ws3): + """A client-submitted delegation chain must be dropped before routing.""" + ws = ws3["workspace"] + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "innocuous text", + extra_meta={ + "delegated_by": "agent-b", + "delegated_to": ["agent-a"], + "receipt_from": ["agent-c"], + }, + )) + assert "delegated_by" not in out.metadata + assert "delegated_to" not in out.metadata + assert "receipt_from" not in out.metadata + + def test_e1_missing(self, db, ws3): + ws = ws3["workspace"] + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=str(uuid.uuid4()), reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_cross_channel_reference(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, target="channel/session-other") + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_delegated_by_source_mismatch(self, db, ws3): + """E1 whose source doesn't match its delegated_by is never honoured.""" + ws = ws3["workspace"] + e1 = _delegation_event(db, ws) + meta = dict(e1.metadata_) + meta["delegated_by"] = "agent-c" # source stays openagents:agent-a + e1.metadata_ = meta + db.flush() + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_replier_not_in_delegated_to(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, delegated_to=("agent-c",)) + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_delegator_left_channel(self, db, ws3): + """Workspace membership isn't enough — A must still be in the channel.""" + ws, ch = ws3["workspace"], ws3["channel"] + e1 = _delegation_event(db, ws) + from sqlalchemy import select + member = db.execute( + select(ChannelMember).where( + ChannelMember.channel_id == ch.id, + ChannelMember.agent_name == "agent-a", + ) + ).scalar_one() + db.delete(member) + db.flush() + db.refresh(ch) + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_expired_delegation(self, db, ws3): + ws = ws3["workspace"] + stale = _now_ms() - int(48 * 3600 * 1000) # older than the 24h default TTL + e1 = _delegation_event(db, ws, timestamp=stale) + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_duplicate_terminal_receipt(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done again", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + db.refresh(e1) + assert e1.metadata_["receipt_from"] == ["agent-b"] # unchanged + + def test_ack_of_receipt_does_not_bounce(self, db, ws3): + """A's reply to B's receipt references E2, which carries no + delegated_by — so it must not deterministically bounce back to B.""" + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + e2_event = _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + ) + _handle(db, ws, e2_event) + # Persist E2 the way PersistenceMod would. + db.add(EventRecord( + id=e2_event.id, network_id=ws.id, type=e2_event.type, + source=e2_event.source, target=e2_event.target, + payload=e2_event.payload, metadata_=e2_event.metadata, + timestamp=e2_event.timestamp, + )) + db.flush() + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-a", "thanks!", in_reply_to=e2_event.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + +class TestMasterMode: + @pytest.fixture + def master_ws(self, db, ws3): + ch = ws3["channel"] + ch.master_agent = "agent-a" + ch.orchestration_mode = "master" + db.flush() + db.refresh(ch) + return ws3 + + def test_receipt_in_master_mode_ignores_onward_mentions(self, db, master_ws): + """Star topology stays authoritative: a sub-agent's receipt cannot + delegate onward, even if it @mentions another sub.""" + ws = master_ws["workspace"] + e1 = _delegation_event(db, ws) # master agent-a delegated to agent-b + out = _handle(db, ws, _reply( + "openagents:agent-b", "done, @agent-c could verify.", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a"] + assert "delegated_by" not in out.metadata + + +class TestDelegationMarking: + def test_master_mention_marks_delegation(self, db, ws3): + """Explicit @mention delegation gets server-written delegated_by/to + (router mocked to route to the mentioned agent).""" + ws = ws3["workspace"] + client_patch, _ = _with_router("next:agent-b") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-a", "@agent-b please handle this.", + )) + assert out.metadata["target_agents"] == ["agent-b"] + assert out.metadata["delegated_by"] == "agent-a" + assert out.metadata["delegated_to"] == ["agent-b"] + + def test_router_inferred_hop_not_marked(self, db, ws3): + """Router routing without a mention (e.g. report to master) is NOT a + delegation — marking it would bounce acknowledgements.""" + ws = ws3["workspace"] + client_patch, _ = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "Reporting back with findings.", + )) + assert out.metadata["target_agents"] == ["agent-a"] + assert "delegated_by" not in out.metadata + + def test_human_message_never_marked(self, db, ws3): + ws = ws3["workspace"] + client_patch, _ = _with_router("next:agent-b") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply("human:user", "@agent-b please help")) + assert out.metadata["target_agents"] == ["agent-b"] + assert "delegated_by" not in out.metadata From 9d7389a7eae0867557771714510f098e59533426 Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 04:50:43 +0000 Subject: [PATCH 2/8] harden delegation receipts: atomic claim, membership checks, at-most-once, scoped badges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up review fixes on the receipt-routing branch: - migrate every remaining turn-ending sendError in aider/amp/cline/copilot/ cursor/gemini/goose/llm-direct/mini to sendFinalError so delegated failures (missing CLI, config errors, spawn/timeout/non-zero exit) still return a deterministic receipt; goose threads the trigger through _runGoose including its retry recursion - claim the single terminal receipt under SELECT ... FOR UPDATE on E1 so concurrent duplicate replies cannot both route to the delegator or lose a receipt_from stamp (SQLite tests unaffected — FOR UPDATE is a no-op) - verify delegator AND replier against live WorkspaceMember rows (hard- and soft-removed) in addition to channel membership: ChannelMember rows outlive workspace removal and could re-target an unreachable agent - duplicate receipts no longer fall back to the LLM router (which could re-select the delegator): the delegator is suppressed deterministically, explicit onward @mentions are still honoured; needs_input is non-consuming so an ask followed by the real result both deliver - stamp reply_kind only when the trigger carries the server-written delegated_by/delegated_to naming this agent — ordinary human-triggered replies no longer render as delegation receipts in Launcher; in_reply_to correlation is kept for every real trigger Tests: backend 29 receipt tests (at-most-once vs router re-route, onward on duplicate, non-consuming needs_input, workspace-removed delegator), connector 787 passing incl. reply_kind gating. --- .../agent-connector/src/adapters/aider.js | 6 +- packages/agent-connector/src/adapters/amp.js | 7 +- packages/agent-connector/src/adapters/base.js | 20 +++-- .../agent-connector/src/adapters/cline.js | 6 +- .../agent-connector/src/adapters/copilot.js | 8 +- .../agent-connector/src/adapters/cursor.js | 4 +- .../agent-connector/src/adapters/gemini.js | 4 +- .../agent-connector/src/adapters/goose.js | 23 +++--- .../src/adapters/llm-direct.js | 3 +- packages/agent-connector/src/adapters/mini.js | 7 +- .../test/terminal-replies.test.js | 46 +++++++++-- workspace/backend/app/mods/workspace_mod.py | 69 ++++++++++++---- .../backend/tests/test_receipt_routing.py | 79 ++++++++++++++++++- 13 files changed, 219 insertions(+), 63 deletions(-) diff --git a/packages/agent-connector/src/adapters/aider.js b/packages/agent-connector/src/adapters/aider.js index acbd2a0fc..e1ba31e16 100644 --- a/packages/agent-connector/src/adapters/aider.js +++ b/packages/agent-connector/src/adapters/aider.js @@ -449,7 +449,7 @@ class AiderAdapter extends BaseAdapter { if (!this._aiderBin) this._aiderBin = this._findAiderBinary(); if (!this._aiderBin) { - await this.sendError(msgChannel, `Aider CLI not found. Install with: ${aiderInstallHint()}`); + await this.sendFinalError(msg, msgChannel, `Aider CLI not found. Install with: ${aiderInstallHint()}`); return; } @@ -458,7 +458,7 @@ class AiderAdapter extends BaseAdapter { // injects the key into the wrong provider). const resolution = this._resolveConfig(); if (resolution.error) { - await this.sendError(msgChannel, `Configuration error: ${resolution.error}`); + await this.sendFinalError(msg, msgChannel, `Configuration error: ${resolution.error}`); return; } @@ -472,7 +472,7 @@ class AiderAdapter extends BaseAdapter { result = await this._runAider(content, msgChannel); } catch (e) { this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); return; } diff --git a/packages/agent-connector/src/adapters/amp.js b/packages/agent-connector/src/adapters/amp.js index 6ce6b7664..6366d5a17 100644 --- a/packages/agent-connector/src/adapters/amp.js +++ b/packages/agent-connector/src/adapters/amp.js @@ -246,7 +246,7 @@ class AmpAdapter extends BaseAdapter { if (!this._ampBin) { const message = `Amp CLI not found — install with: ${ampInstallHint()}`; this._reportStatus(REASON.RUNTIME_MISSING, message); - await this.sendError(msgChannel, message); + await this.sendFinalError(msg, msgChannel, message); return; } @@ -269,10 +269,11 @@ class AmpAdapter extends BaseAdapter { }); this._log(message); this._reportStatus(reason, message); - await this.sendError(msgChannel, message); + await this.sendFinalError(msg, msgChannel, message); } else { this._log(`Error handling message: ${redactDiagnostic(e.message)}`); - await this.sendError( + await this.sendFinalError( + msg, msgChannel, `Error processing message: ${redactDiagnostic(e.message)}`, ); diff --git a/packages/agent-connector/src/adapters/base.js b/packages/agent-connector/src/adapters/base.js index 1fcb867ec..7a136657d 100644 --- a/packages/agent-connector/src/adapters/base.js +++ b/packages/agent-connector/src/adapters/base.js @@ -862,15 +862,25 @@ class BaseAdapter { // delegated task's outcome back to the delegating agent instead of // relying on the LLM router to guess "this is a report". `triggerMsg` is // the inbound message that started the turn. Synthetic triggers - // (system:* senders, missing event id) degrade to a plain reply — the - // backend only honours receipts that reference a real delegation event, - // so over-stamping is harmless but under-stamping loses the receipt. + // (system:* senders, missing event id) degrade to a plain reply. + // + // reply_kind is only stamped when the trigger carries the SERVER-written + // delegation fields naming this agent: the UI renders its receipt badge + // purely from reply_kind, so stamping ordinary human-triggered replies + // would present them as delegation receipts. in_reply_to alone is kept + // for every real trigger — it is plain correlation data. _receiptMeta(kind, triggerMsg) { - const meta = { reply_kind: kind }; + const meta = {}; const id = triggerMsg && triggerMsg.messageId; const sender = (triggerMsg && triggerMsg.senderName) || ''; - if (id && !sender.startsWith('system:')) meta.in_reply_to = id; + if (!id || sender.startsWith('system:')) return meta; + meta.in_reply_to = id; + const tm = (triggerMsg && triggerMsg.metadata) || {}; + const delegatedTo = Array.isArray(tm.delegated_to) ? tm.delegated_to : []; + if (tm.delegated_by && delegatedTo.includes(this.agentName)) { + meta.reply_kind = kind; + } return meta; } diff --git a/packages/agent-connector/src/adapters/cline.js b/packages/agent-connector/src/adapters/cline.js index e7aa455cd..e3b165a3f 100644 --- a/packages/agent-connector/src/adapters/cline.js +++ b/packages/agent-connector/src/adapters/cline.js @@ -485,13 +485,13 @@ class ClineAdapter extends BaseAdapter { // back to the launcher/repo dir — return a clear error instead. const workingDir = this.workingDir || defaultAgentWorkdir(this.agentName); if (this.workingDir && !this._dirExists(this.workingDir)) { - await this.sendError(channel, `Working directory does not exist: ${this.workingDir}`); + await this.sendFinalError(msg, channel, `Working directory does not exist: ${this.workingDir}`); return; } const clineBin = this._findClineBinary(); if (!clineBin) { - await this.sendError(channel, + await this.sendFinalError(msg, channel, 'Cline CLI not found. Install it with: npm install -g cline'); return; } @@ -501,7 +501,7 @@ class ClineAdapter extends BaseAdapter { const ver = this._checkClineVersion(clineBin); if (ver.compatible === false) { this._log(`Refusing to start: Cline ${ver.version} < minimum ${MIN_CLINE_VERSION}`); - await this.sendError(channel, + await this.sendFinalError(msg, channel, `Cline CLI ${ver.version} is below the minimum supported version ${MIN_CLINE_VERSION}. ` + 'Please upgrade with: npm install -g cline@latest'); return; diff --git a/packages/agent-connector/src/adapters/copilot.js b/packages/agent-connector/src/adapters/copilot.js index f5b1bbfe8..a1d87da46 100644 --- a/packages/agent-connector/src/adapters/copilot.js +++ b/packages/agent-connector/src/adapters/copilot.js @@ -417,7 +417,7 @@ class CopilotAdapter extends BaseAdapter { this._log(`Processing message from ${sender} in ${channel}: ${redactSensitive(content).slice(0, 80)}...`); if (!this._copilotBin) { - await this.sendError(channel, + await this.sendFinalError(msg, channel, 'GitHub Copilot CLI not found. Install it with: npm install -g @github/copilot'); return; } @@ -427,7 +427,7 @@ class CopilotAdapter extends BaseAdapter { // "unknown option" failure on every run. const gate = this._checkVersionGate(); if (gate.compatible === false) { - await this.sendError(channel, + await this.sendFinalError(msg, channel, `GitHub Copilot CLI ${gate.version} is too old — this integration requires ${MIN_VERSION} or newer. Upgrade with: copilot update (or npm install -g @github/copilot).`); return; } @@ -435,7 +435,7 @@ class CopilotAdapter extends BaseAdapter { // Working directory must exist — never silently fall back to the repo cwd. const wd = this.workingDir; if (wd && !this._dirExists(wd)) { - await this.sendError(channel, `Working directory does not exist: ${wd}`); + await this.sendFinalError(msg, channel, `Working directory does not exist: ${wd}`); return; } @@ -454,7 +454,7 @@ class CopilotAdapter extends BaseAdapter { try { result = await this._runTurn(channel, args); } catch (e) { - await this.sendError(channel, `Error: ${redactSensitive(e.message)}`); + await this.sendFinalError(msg, channel, `Error: ${redactSensitive(e.message)}`); return; } diff --git a/packages/agent-connector/src/adapters/cursor.js b/packages/agent-connector/src/adapters/cursor.js index 7433e44b3..a4c68b948 100644 --- a/packages/agent-connector/src/adapters/cursor.js +++ b/packages/agent-connector/src/adapters/cursor.js @@ -436,7 +436,7 @@ class CursorAdapter extends BaseAdapter { try { cmd = this._buildCursorCmd(effectiveContent, msgChannel, { skipResume: attempt > 0 }); } catch (e) { - await this.sendError(msgChannel, e.message); + await this.sendFinalError(msg, msgChannel, e.message); return; } @@ -645,7 +645,7 @@ class CursorAdapter extends BaseAdapter { }); } catch (e) { this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); break; } if (!_shouldRetry) break; diff --git a/packages/agent-connector/src/adapters/gemini.js b/packages/agent-connector/src/adapters/gemini.js index d027a49fb..996e1ba97 100644 --- a/packages/agent-connector/src/adapters/gemini.js +++ b/packages/agent-connector/src/adapters/gemini.js @@ -297,7 +297,7 @@ class GeminiAdapter extends BaseAdapter { const built = this._buildGeminiCmd(content, msgChannel, { skipResume: attempt > 0 }); cmd = built.cmd; } catch (e) { - await this.sendError(msgChannel, e.message); + await this.sendFinalError(msg, msgChannel, e.message); return; } @@ -460,7 +460,7 @@ class GeminiAdapter extends BaseAdapter { }); } catch (e) { this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${e.message}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${e.message}`); break; } if (!_shouldRetry) break; diff --git a/packages/agent-connector/src/adapters/goose.js b/packages/agent-connector/src/adapters/goose.js index 3b9973d47..3651ee3f3 100644 --- a/packages/agent-connector/src/adapters/goose.js +++ b/packages/agent-connector/src/adapters/goose.js @@ -364,7 +364,7 @@ class GooseAdapter extends BaseAdapter { try { // null → stopped or a failure already reported; '' → empty success; str → answer. - const result = await this._runGoose(content, msgChannel); + const result = await this._runGoose(content, msgChannel, false, msg); if (result === null) return; if (result) { await this.sendFinalResult(msg, msgChannel, result); @@ -383,7 +383,7 @@ class GooseAdapter extends BaseAdapter { return; } this._log(`Error handling message: ${e.message}`); - await this.sendError(msgChannel, `Error processing message: ${this._safe(e.message)}`); + await this.sendFinalError(msg, msgChannel, `Error processing message: ${this._safe(e.message)}`); } finally { this._stoppingChannels.delete(msgChannel); } @@ -442,17 +442,17 @@ class GooseAdapter extends BaseAdapter { return this._versionTooOld; } - _runGoose(content, channel, retry = false) { + _runGoose(content, channel, retry = false, triggerMsg = null) { let cwd; try { cwd = this._resolveCwd(); } catch (e) { - return this.sendError(channel, e.message).then(() => null, () => null); + return this.sendFinalError(triggerMsg, channel, e.message).then(() => null, () => null); } // Refuse a Goose CLI older than the verified-stable minimum. const tooOld = this._versionTooOldMessage(); - if (tooOld) return this.sendError(channel, tooOld).then(() => null, () => null); + if (tooOld) return this.sendFinalError(triggerMsg, channel, tooOld).then(() => null, () => null); const sessionName = this._channelSessions[channel] || gooseSessionName(this.workspaceId, this.agentName, channel); @@ -463,7 +463,7 @@ class GooseAdapter extends BaseAdapter { try { cmd = this._buildCmd(sessionName, resume, systemPrompt); } catch (e) { - return this.sendError(channel, e.message).then(() => null, () => null); + return this.sendFinalError(triggerMsg, channel, e.message).then(() => null, () => null); } const env = this._buildEnv(); @@ -486,7 +486,7 @@ class GooseAdapter extends BaseAdapter { windowsHide: true, }); } catch (e) { - this.sendError(channel, `Failed to start Goose: ${this._safe(e.message)}`) + this.sendFinalError(triggerMsg, channel, `Failed to start Goose: ${this._safe(e.message)}`) .then(() => resolve(null), () => resolve(null)); return; } @@ -516,7 +516,8 @@ class GooseAdapter extends BaseAdapter { this._log(`Goose produced no output for ${timeoutSec}s — treating as hung, killing.`); this._stoppingChannels.add(channel); this._stopProcess(proc).catch(() => {}); - this.sendError( + this.sendFinalError( + triggerMsg, channel, 'Goose appears to have hung (no output for a long time) and was stopped. ' + 'Try a smaller task or check the provider.', @@ -552,7 +553,7 @@ class GooseAdapter extends BaseAdapter { settled = true; clearInterval(watchdog); if (this._channelProcesses[channel] === proc) delete this._channelProcesses[channel]; - this.sendError(channel, `Failed to run Goose: ${this._safe(err.message)}`) + this.sendFinalError(triggerMsg, channel, `Failed to run Goose: ${this._safe(err.message)}`) .then(() => resolve(null), () => resolve(null)); }); @@ -583,7 +584,7 @@ class GooseAdapter extends BaseAdapter { channel, 'Previous Goose session was unavailable — starting a new one (earlier context is reset).', ); - resolve(await this._runGoose(content, channel, true)); + resolve(await this._runGoose(content, channel, true, triggerMsg)); return; } @@ -593,7 +594,7 @@ class GooseAdapter extends BaseAdapter { const detail = parser.errorMessage || stderrText; const message = classifyGooseError(detail) || (detail ? this._safe(detail).slice(0, 500) : `Goose exited with code ${code}.`); - await this.sendError(channel, this._safe(message)); + await this.sendFinalError(triggerMsg, channel, this._safe(message)); resolve(null); return; } diff --git a/packages/agent-connector/src/adapters/llm-direct.js b/packages/agent-connector/src/adapters/llm-direct.js index 82a2caa14..89cf2a910 100644 --- a/packages/agent-connector/src/adapters/llm-direct.js +++ b/packages/agent-connector/src/adapters/llm-direct.js @@ -98,7 +98,8 @@ class LlmDirectAdapter extends BaseAdapter { try { if (!this._directMode) { - await this.sendError( + await this.sendFinalError( + msg, msgChannel, `${this._adapterLabel} direct API mode not configured. Set OPENAI_API_KEY + OPENAI_BASE_URL.` ); diff --git a/packages/agent-connector/src/adapters/mini.js b/packages/agent-connector/src/adapters/mini.js index 3b03d2fb7..d67aff13f 100644 --- a/packages/agent-connector/src/adapters/mini.js +++ b/packages/agent-connector/src/adapters/mini.js @@ -355,7 +355,7 @@ class MiniSweAgentAdapter extends BaseAdapter { if (!this._miniBin) { const message = `mini-SWE-agent CLI not found — install with: ${miniInstallHint()}`; this._reportStatus(REASON.RUNTIME_MISSING, message); - await this.sendError(msgChannel, message); + await this.sendFinalError(msg, msgChannel, message); return; } @@ -376,10 +376,11 @@ class MiniSweAgentAdapter extends BaseAdapter { }); this._log(message); this._reportStatus(reason, message); - await this.sendError(msgChannel, message); + await this.sendFinalError(msg, msgChannel, message); } else { this._log(`Error handling message: ${redactDiagnostic(e.message)}`); - await this.sendError( + await this.sendFinalError( + msg, msgChannel, `Error processing message: ${redactDiagnostic(e.message)}`, ); diff --git a/packages/agent-connector/test/terminal-replies.test.js b/packages/agent-connector/test/terminal-replies.test.js index 0600e5bd7..146405436 100644 --- a/packages/agent-connector/test/terminal-replies.test.js +++ b/packages/agent-connector/test/terminal-replies.test.js @@ -33,7 +33,12 @@ function mkAdapter(overrides = {}) { return adapter; } -const TRIGGER = { messageId: 'evt-123', senderType: 'agent', senderName: 'delegator-a', content: 'do X' }; +// A server-marked delegation trigger: the backend wrote delegated_by/to onto +// the routed event, naming this agent ('tester') as the delegate. +const TRIGGER = { + messageId: 'evt-123', senderType: 'agent', senderName: 'delegator-a', content: 'do X', + metadata: { delegated_by: 'delegator-a', delegated_to: ['tester'] }, +}; describe('terminal reply metadata', () => { it('stamps reply_kind and in_reply_to for a real trigger', async () => { @@ -58,22 +63,53 @@ describe('terminal reply metadata', () => { for (const s of a.sent) assert.equal(s.opts.metadata.in_reply_to, 'evt-123'); }); - it('omits in_reply_to for synthetic system triggers', async () => { + it('stamps nothing for synthetic system triggers', async () => { const a = mkAdapter(); await a.sendFinalResult( { messageId: 'evt-9', senderName: 'system:todos', content: 'nudge' }, 'general', 'done', ); const meta = a.sent[0].opts.metadata; - assert.equal(meta.reply_kind, 'result'); + assert.equal(meta.reply_kind, undefined); assert.equal(meta.in_reply_to, undefined); }); - it('omits in_reply_to when the trigger is missing or has no event id', async () => { + it('stamps nothing when the trigger is missing or has no event id', async () => { const a = mkAdapter(); await a.sendCancelled(null, 'general', 'stopped'); await a.sendFinalError({ senderName: 'someone' }, 'general', 'boom'); - for (const s of a.sent) assert.equal(s.opts.metadata.in_reply_to, undefined); + for (const s of a.sent) { + assert.equal(s.opts.metadata.in_reply_to, undefined); + assert.equal(s.opts.metadata.reply_kind, undefined); + } + }); + + it('keeps in_reply_to but omits reply_kind for non-delegated triggers', async () => { + // Ordinary human-triggered turns must not render as delegation receipts + // in the UI (the badge keys off reply_kind alone), but the correlation + // id is still useful. + const a = mkAdapter(); + await a.sendFinalResult( + { messageId: 'evt-h1', senderType: 'human', senderName: 'alice', content: 'hi' }, + 'general', 'hello', + ); + const meta = a.sent[0].opts.metadata; + assert.equal(meta.in_reply_to, 'evt-h1'); + assert.equal(meta.reply_kind, undefined); + }); + + it('omits reply_kind when the delegation names a different agent', async () => { + const a = mkAdapter(); + await a.sendFinalResult( + { + messageId: 'evt-x', senderType: 'agent', senderName: 'delegator-a', content: 'do X', + metadata: { delegated_by: 'delegator-a', delegated_to: ['someone-else'] }, + }, + 'general', 'done', + ); + const meta = a.sent[0].opts.metadata; + assert.equal(meta.in_reply_to, 'evt-x'); + assert.equal(meta.reply_kind, undefined); }); it('swallows send failures for error kind but propagates for result kind', async () => { diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index efe5d0f2b..501ab315c 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -664,16 +664,23 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> router and is routed straight to the delegating agent, so a delegated task's outcome can never be swallowed by a router "stop". - Returns ``(targets, onward_delegates)`` on a hit — ``targets`` always + Returns ``(targets, onward_delegates)`` on a hit — ``targets`` normally starts with the delegator; ``onward_delegates`` are additional agents the reply @mentions (an onward delegation, dual-routed so the delegator still sees the outcome). Returns ``None`` when the message is not a valid receipt, in which case the caller falls through to normal routing. - Each (E1, replier) pair gets exactly one receipt: on a hit E1 is stamped - with ``receipt_from`` so a later duplicate falls back to normal routing. - If the delegator needs more work done it delegates again, creating a new - E1. + Delivery to the delegator is at-most-once per (E1, replier): the first + terminal receipt stamps E1 with ``receipt_from`` (under a row lock, so + concurrent duplicates cannot both claim it), and a later duplicate is + still handled deterministically but with the delegator SUPPRESSED — + returning it to the LLM router could re-route to the delegator and break + the at-most-once guarantee. An explicit onward @mention in the duplicate + is still honoured. ``needs_input`` receipts are non-consuming: an agent + may surface a question mid-turn and still owe the real result (e.g. + Cline's ask event followed by the final text), so only result / error / + cancelled claim the single terminal slot. If the delegator needs more + work done it delegates again, creating a new E1. """ from app.config import config from app.models import EventRecord @@ -690,11 +697,16 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> if not ref or not isinstance(ref, str): return None + # Row-lock E1 for the rest of the transaction: the duplicate check and + # the receipt_from stamp below are a read-modify-write, and two replies + # referencing the same E1 processed concurrently must not both claim the + # single terminal receipt. SQLite (tests) ignores FOR UPDATE; Postgres + # serializes the claim. e1 = db.execute( select(EventRecord).where( EventRecord.id == ref, EventRecord.network_id == workspace.id, - ) + ).with_for_update() ).scalar_one_or_none() if e1 is None or e1.type != WorkspaceEventTypes.MESSAGE_POSTED: return None @@ -719,21 +731,28 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> if delegator not in participants or replier not in participants: return None - ttl_ms = int(config.DELEGATION_RECEIPT_TTL_HOURS * 3600 * 1000) - if e1.timestamp and (int(time.time() * 1000) - int(e1.timestamp)) > ttl_ms: + # ... and of the workspace itself. ChannelMember rows are not cleaned up + # when a workspace member is removed (remove_member deletes only the + # WorkspaceMember row; network removal soft-deletes with status + # "removed"), so a stale channel participant could otherwise become a + # receipt target that can no longer poll the workspace. + from app.models import WorkspaceMember + live_members = { + m.agent_name for m in db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == workspace.id, + WorkspaceMember.agent_name.in_([delegator, replier]), + ) + ).scalars().all() + if (m.status or "") != "removed" + } + if delegator not in live_members or replier not in live_members: return None - already = e1_meta.get("receipt_from") or [] - if replier in already: + ttl_ms = int(config.DELEGATION_RECEIPT_TTL_HOURS * 3600 * 1000) + if e1.timestamp and (int(time.time() * 1000) - int(e1.timestamp)) > ttl_ms: return None - # Stamp E1 (single terminal receipt per replier). A fresh dict is - # assigned so SQLAlchemy detects the JSONB change. - stamped = dict(e1_meta) - stamped["receipt_from"] = already + [replier] - e1.metadata_ = stamped - db.flush() - # Onward delegation: mentions other than the original delegator. A "@A # I'm done" style report must NOT count as a new delegation back to A — # that would bounce A's acknowledgement to the replier and re-create the @@ -749,6 +768,22 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> onward.append(m) seen.add(m) + already = e1_meta.get("receipt_from") or [] + if replier in already: + # Duplicate terminal receipt: the delegator already got their one + # delivery. Suppress them deterministically (handing the message to + # the LLM router could pick the delegator again), but honour an + # explicit onward delegation. Empty targets → caller's sentinel. + return list(onward), onward + + # Claim the single terminal receipt — except for needs_input, which is + # an intermediate "waiting on you" signal, not the turn's outcome. + if meta.get("reply_kind") != "needs_input": + stamped = dict(e1_meta) + stamped["receipt_from"] = already + [replier] + e1.metadata_ = stamped + db.flush() + return [delegator] + onward, onward diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py index f6ed83abb..41fb67aac 100644 --- a/workspace/backend/tests/test_receipt_routing.py +++ b/workspace/backend/tests/test_receipt_routing.py @@ -280,16 +280,87 @@ def test_expired_delegation(self, db, ws3): )) assert out.metadata["target_agents"] == ["__no_response__"] - def test_duplicate_terminal_receipt(self, db, ws3): + def test_duplicate_terminal_receipt_suppressed_without_router(self, db, ws3): + """At-most-once: a duplicate must NOT fall back to the LLM router, + which could re-select the delegator — it is suppressed outright.""" ws = ws3["workspace"] e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) - out, _ = self._stop_routed(db, ws, _reply( - "openagents:agent-b", "done again", in_reply_to=e1.id, reply_kind="result", - )) + client_patch, mock_client = _with_router("next:agent-a") # router WOULD re-route + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done again", in_reply_to=e1.id, reply_kind="result", + )) assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() db.refresh(e1) assert e1.metadata_["receipt_from"] == ["agent-b"] # unchanged + def test_duplicate_receipt_still_honours_onward_mention(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) + out = _handle(db, ws, _reply( + "openagents:agent-b", "also @agent-c please verify", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-c"] + assert out.metadata["delegated_by"] == "agent-b" + assert out.metadata["delegated_to"] == ["agent-c"] + + def test_needs_input_is_non_consuming(self, db, ws3): + """An agent may ask a question mid-task and still owe the result: + needs_input routes to the delegator but does not claim the single + terminal receipt.""" + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + out = _handle(db, ws, _reply( + "openagents:agent-b", "which env?", in_reply_to=e1.id, reply_kind="needs_input", + )) + assert out.metadata["target_agents"] == ["agent-a"] + db.refresh(e1) + assert "receipt_from" not in (e1.metadata_ or {}) + # The real terminal result afterwards still lands deterministically. + out2 = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out2.metadata["target_agents"] == ["agent-a"] + db.refresh(e1) + assert e1.metadata_["receipt_from"] == ["agent-b"] + + def test_delegator_removed_from_workspace_but_still_in_channel(self, db, ws3): + """ChannelMember rows outlive WorkspaceMember removal — a receipt must + not target an agent that can no longer poll the workspace.""" + ws = ws3["workspace"] + e1 = _delegation_event(db, ws) + from sqlalchemy import select + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-a", + ) + ).scalar_one() + db.delete(member) + db.flush() + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + + def test_delegator_soft_removed_from_workspace(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws) + from sqlalchemy import select + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-a", + ) + ).scalar_one() + member.status = "removed" + db.flush() + out, _ = self._stop_routed(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + def test_ack_of_receipt_does_not_bounce(self, db, ws3): """A's reply to B's receipt references E2, which carries no delegated_by — so it must not deterministically bounce back to B.""" From 19204bf266fa2ed2b8a4372adb479712d77a8af1 Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 06:07:26 +0000 Subject: [PATCH 3/8] claude: build-failure path posts a terminal error receipt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _buildClaudeCmd throws when the Claude CLI is unavailable or turn configuration cannot be built; that catch ends the turn, so a delegated turn must return a deterministic error receipt instead of a plain sendError. The watchdog notice stays non-terminal — its path resolves into retry/exit handling that posts the final outcome itself. --- packages/agent-connector/src/adapters/claude.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/agent-connector/src/adapters/claude.js b/packages/agent-connector/src/adapters/claude.js index 8455922bd..a13de356f 100644 --- a/packages/agent-connector/src/adapters/claude.js +++ b/packages/agent-connector/src/adapters/claude.js @@ -1296,7 +1296,7 @@ class ClaudeAdapter extends BaseAdapter { cmd = built.cmd; mcpConfigFile = built.mcpConfigFile; } catch (e) { - await this.sendError(msgChannel, e.message); + await this.sendFinalError(msg, msgChannel, e.message); return; } From a28751b3b13b1a3bcd161ca0f32bfbf48a0a97bc Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 06:21:37 +0000 Subject: [PATCH 4/8] suppress undeliverable receipts instead of falling back into orchestration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An authenticated receipt whose delegator is no longer deliverable (left the channel, hard- or soft-removed from the workspace) used to return None from _receipt_route, i.e. "not a receipt" — normal routing resumed and _master_targets would route the sub-agent's reply straight back to the stale master (channel.master_agent survives every removal path), bypassing the very guard that had just failed. The LLM router could do the same in dynamic mode. Membership failures and consumed duplicates now return (onward, onward): the delegator is suppressed deterministically (empty targets → sentinel) while an explicit onward @mention is still honoured, and orchestration is never re-entered. Deliberate deviation from the review on TTL expiry: it still falls back to normal routing. At that point the delegator is verified alive and present, so best-effort orchestration (master star rule, router "reports back") can still deliver a long-running task's late result — suppressing would guarantee exactly the loss this feature exists to prevent. The TTL only stops stale E1s from forcing deterministic routing; an expired E1 is never claimed. Duplicate check moved before the TTL so at-most-once holds regardless of E1 age. Tests: stale-master suppression across all three removal paths in master mode, expired-E1 star delivery to a live master, onward mention surviving suppression, consumed+removed combination, plain sub-agent chatter still star-routed; connector regression for _buildClaudeCmd throwing on a delegated turn (reply_kind=error receipt). Backend 31 receipt cases, connector 788 passing. --- .../test/terminal-replies.test.js | 38 +++++ workspace/backend/app/mods/workspace_mod.py | 74 +++++---- .../backend/tests/test_receipt_routing.py | 147 ++++++++++++++++-- 3 files changed, 217 insertions(+), 42 deletions(-) diff --git a/packages/agent-connector/test/terminal-replies.test.js b/packages/agent-connector/test/terminal-replies.test.js index 146405436..5ef538e16 100644 --- a/packages/agent-connector/test/terminal-replies.test.js +++ b/packages/agent-connector/test/terminal-replies.test.js @@ -120,6 +120,44 @@ describe('terminal reply metadata', () => { }); }); +describe('claude build failure', () => { + it('posts an error receipt when _buildClaudeCmd throws on a delegated turn', async () => { + const ClaudeAdapter = require('../src/adapters/claude'); + const a = new ClaudeAdapter({ + workspaceId: 'ws-x', channelName: 'general', token: 'tok', agentName: 'tester', + disabledModules: new Set(['knowledge']), + }); + const sent = []; + // Any client method the pre-build path touches resolves benignly; only + // sendMessage records what actually got posted. + a.client = new Proxy({}, { + get: (_t, prop) => prop === 'sendMessage' + ? async (_w, _c, _tok, content, opts) => { sent.push({ content, opts }); } + : async () => [], + }); + a._saveSessions = () => {}; + a.sendStatus = async () => {}; + a.sendThinking = async () => {}; + a.getRemainingTodos = async () => []; + a.getBrowserEnabled = async () => false; + a._resetIdleTimer = () => {}; + a._titledSessions.add('general'); + a._buildClaudeCmd = () => { throw new Error('Claude CLI not found'); }; + + await a._handleMessage({ + messageId: 'evt-del-1', senderType: 'agent', senderName: 'delegator-a', + sessionId: 'general', content: 'do X', + metadata: { delegated_by: 'delegator-a', delegated_to: ['tester'] }, + }); + + const receipt = sent.find((s) => s.opts && s.opts.metadata && s.opts.metadata.reply_kind); + assert.ok(receipt, 'expected a stamped terminal message'); + assert.equal(receipt.opts.metadata.reply_kind, 'error'); + assert.equal(receipt.opts.metadata.in_reply_to, 'evt-del-1'); + assert.match(receipt.content, /Claude CLI not found/); + }); +}); + describe('inflight turn registry', () => { it('tracks the current message across the queue drain and clears at the end', async () => { const a = mkAdapter(); diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index 501ab315c..19ed9893b 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -667,8 +667,13 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> Returns ``(targets, onward_delegates)`` on a hit — ``targets`` normally starts with the delegator; ``onward_delegates`` are additional agents the reply @mentions (an onward delegation, dual-routed so the delegator still - sees the outcome). Returns ``None`` when the message is not a valid - receipt, in which case the caller falls through to normal routing. + sees the outcome). Returns ``None`` ONLY when the message is not an + authenticated receipt (or its E1 is past the TTL — see below), in which + case the caller falls through to normal routing. An authenticated receipt + whose delegator is undeliverable (left the channel, removed from the + workspace) or already served returns ``(onward, onward)`` — possibly + empty — so it can never re-enter master/LLM orchestration and be + misrouted back to a stale delegator. Delivery to the delegator is at-most-once per (E1, replier): the first terminal receipt stamps E1 with ``receipt_from`` (under a row lock, so @@ -725,11 +730,36 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> if replier not in (e1_meta.get("delegated_to") or []): return None + # The delegation chain is authenticated from here on. Any further check + # that fails means the receipt is REAL but must not be delivered to the + # delegator — and it must NOT fall back into normal orchestration either: + # _master_targets would route a sub-agent's message straight back to a + # removed/stale master and the LLM router can re-select the delegator, + # bypassing exactly the guard that just failed. Those cases return + # (onward, onward): the delegator is suppressed (empty targets → the + # caller's no-response sentinel) while an explicit onward @mention is + # still honoured. + participants = {p.agent_name for p in (channel.participants or [])} + + # Onward delegation: mentions other than the original delegator. A "@A + # I'm done" style report must NOT count as a new delegation back to A — + # that would bounce A's acknowledgement to the replier and re-create the + # ping-pong this whole mechanism exists to prevent. In master mode the + # star topology stays authoritative: sub-agents cannot delegate onward, + # so mentions are ignored (matching _master_targets). + mode = (getattr(channel, "orchestration_mode", None) or "dynamic").lower() + onward: List[str] = [] + if mode != "master": + seen = {delegator, replier} + for m in mentions: + if m not in seen and m in participants: + onward.append(m) + seen.add(m) + # Both ends must still be members of this channel — workspace membership # alone would let a receipt re-target someone who already left the room. - participants = {p.agent_name for p in (channel.participants or [])} if delegator not in participants or replier not in participants: - return None + return list(onward), onward # ... and of the workspace itself. ChannelMember rows are not cleaned up # when a workspace member is removed (remove_member deletes only the @@ -747,35 +777,25 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> if (m.status or "") != "removed" } if delegator not in live_members or replier not in live_members: - return None - - ttl_ms = int(config.DELEGATION_RECEIPT_TTL_HOURS * 3600 * 1000) - if e1.timestamp and (int(time.time() * 1000) - int(e1.timestamp)) > ttl_ms: - return None - - # Onward delegation: mentions other than the original delegator. A "@A - # I'm done" style report must NOT count as a new delegation back to A — - # that would bounce A's acknowledgement to the replier and re-create the - # ping-pong this whole mechanism exists to prevent. In master mode the - # star topology stays authoritative: sub-agents cannot delegate onward, - # so mentions are ignored (matching _master_targets). - mode = (getattr(channel, "orchestration_mode", None) or "dynamic").lower() - onward: List[str] = [] - if mode != "master": - seen = {delegator, replier} - for m in mentions: - if m not in seen and m in participants: - onward.append(m) - seen.add(m) + return list(onward), onward already = e1_meta.get("receipt_from") or [] if replier in already: # Duplicate terminal receipt: the delegator already got their one - # delivery. Suppress them deterministically (handing the message to - # the LLM router could pick the delegator again), but honour an - # explicit onward delegation. Empty targets → caller's sentinel. + # delivery (at-most-once holds regardless of E1 age, so this check + # sits before the TTL). return list(onward), onward + # TTL expiry deliberately falls back to NORMAL routing rather than + # suppression: at this point the delegator is verified alive and present, + # so best-effort orchestration (master star rule, LLM "reports back") + # can still deliver a long-running task's result — suppressing would + # guarantee the loss this feature exists to prevent. The TTL's job is + # only to stop stale E1s from FORCING deterministic routing. + ttl_ms = int(config.DELEGATION_RECEIPT_TTL_HOURS * 3600 * 1000) + if e1.timestamp and (int(time.time() * 1000) - int(e1.timestamp)) > ttl_ms: + return None + # Claim the single terminal receipt — except for needs_input, which is # an intermediate "waiting on you" signal, not the turn's outcome. if meta.get("reply_kind") != "needs_input": diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py index 41fb67aac..bacf09f55 100644 --- a/workspace/backend/tests/test_receipt_routing.py +++ b/workspace/backend/tests/test_receipt_routing.py @@ -252,8 +252,10 @@ def test_replier_not_in_delegated_to(self, db, ws3): )) assert out.metadata["target_agents"] == ["__no_response__"] - def test_delegator_left_channel(self, db, ws3): - """Workspace membership isn't enough — A must still be in the channel.""" + def test_delegator_left_channel_is_suppressed_not_rerouted(self, db, ws3): + """An authenticated receipt whose delegator left the channel must be + suppressed outright — falling back to orchestration could route it + straight back to the stale delegator.""" ws, ch = ws3["workspace"], ws3["channel"] e1 = _delegation_event(db, ws) from sqlalchemy import select @@ -266,19 +268,30 @@ def test_delegator_left_channel(self, db, ws3): db.delete(member) db.flush() db.refresh(ch) - out, _ = self._stop_routed(db, ws, _reply( - "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", - )) + client_patch, mock_client = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() - def test_expired_delegation(self, db, ws3): + def test_expired_delegation_falls_back_to_best_effort_routing(self, db, ws3): + """TTL expiry with a LIVE delegator falls through to normal routing: + the router may still deliver a long task's result best-effort. + Suppression here would guarantee the loss this feature prevents.""" ws = ws3["workspace"] stale = _now_ms() - int(48 * 3600 * 1000) # older than the 24h default TTL e1 = _delegation_event(db, ws, timestamp=stale) - out, _ = self._stop_routed(db, ws, _reply( - "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", - )) - assert out.metadata["target_agents"] == ["__no_response__"] + client_patch, mock_client = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a"] + mock_client.messages.create.assert_called() + db.refresh(e1) + assert "receipt_from" not in (e1.metadata_ or {}) # expired E1 never claimed def test_duplicate_terminal_receipt_suppressed_without_router(self, db, ws3): """At-most-once: a duplicate must NOT fall back to the LLM router, @@ -339,10 +352,13 @@ def test_delegator_removed_from_workspace_but_still_in_channel(self, db, ws3): ).scalar_one() db.delete(member) db.flush() - out, _ = self._stop_routed(db, ws, _reply( - "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", - )) + client_patch, mock_client = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() def test_delegator_soft_removed_from_workspace(self, db, ws3): ws = ws3["workspace"] @@ -356,10 +372,54 @@ def test_delegator_soft_removed_from_workspace(self, db, ws3): ).scalar_one() member.status = "removed" db.flush() - out, _ = self._stop_routed(db, ws, _reply( - "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + client_patch, mock_client = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() + + def test_undeliverable_receipt_still_honours_onward_mention(self, db, ws3): + """Suppressing a stale delegator must not swallow an explicit onward + delegation in the same reply.""" + ws = ws3["workspace"] + e1 = _delegation_event(db, ws) + from sqlalchemy import select + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-a", + ) + ).scalar_one() + db.delete(member) + db.flush() + out = _handle(db, ws, _reply( + "openagents:agent-b", "done — @agent-c please review", + in_reply_to=e1.id, reply_kind="result", )) + assert out.metadata["target_agents"] == ["agent-c"] + assert out.metadata["delegated_by"] == "agent-b" + + def test_consumed_receipt_with_removed_delegator_stays_suppressed(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) + from sqlalchemy import select + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-a", + ) + ).scalar_one() + db.delete(member) + db.flush() + client_patch, mock_client = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done again", in_reply_to=e1.id, reply_kind="result", + )) assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() def test_ack_of_receipt_does_not_bounce(self, db, ws3): """A's reply to B's receipt references E2, which carries no @@ -405,6 +465,63 @@ def test_receipt_in_master_mode_ignores_onward_mentions(self, db, master_ws): assert out.metadata["target_agents"] == ["agent-a"] assert "delegated_by" not in out.metadata + def _remove_master(self, db, master_ws, how): + from sqlalchemy import select + ws, ch = master_ws["workspace"], master_ws["channel"] + if how == "left_channel": + member = db.execute( + select(ChannelMember).where( + ChannelMember.channel_id == ch.id, + ChannelMember.agent_name == "agent-a", + ) + ).scalar_one() + db.delete(member) + else: + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-a", + ) + ).scalar_one() + if how == "hard": + db.delete(member) + else: + member.status = "removed" + db.flush() + db.refresh(ch) + + @pytest.mark.parametrize("how", ["left_channel", "hard", "soft"]) + def test_stale_master_receipt_suppressed_not_star_routed(self, db, master_ws, how): + """The star rule "sub-agent spoke → back to master" must NOT resurrect + a receipt whose master is gone — channel.master_agent survives every + removal path.""" + ws = master_ws["workspace"] + e1 = _delegation_event(db, ws) # delegated while agent-a was master + self._remove_master(db, master_ws, how) + out = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"], how + + def test_expired_receipt_in_master_mode_star_routes_to_live_master(self, db, master_ws): + """TTL fall-through in master mode: the star rule delivers the late + result to the (verified live) master — best-effort beats loss.""" + ws = master_ws["workspace"] + stale = _now_ms() - int(48 * 3600 * 1000) + e1 = _delegation_event(db, ws, timestamp=stale) + out = _handle(db, ws, _reply( + "openagents:agent-b", "done at last", in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a"] + + def test_plain_sub_agent_message_still_returns_to_master(self, db, master_ws): + """Non-receipt agent chatter keeps the existing star behaviour.""" + ws = master_ws["workspace"] + out = _handle(db, ws, _reply( + "openagents:agent-b", "making progress on the task", + )) + assert out.metadata["target_agents"] == ["agent-a"] + class TestDelegationMarking: def test_master_mention_marks_delegation(self, db, ws3): From eaaac273e6c55edf10036621f4cedf9caa030613 Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 06:45:13 +0000 Subject: [PATCH 5/8] receipts: claim late results, filter onward targets by live membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two edge cases from review of the suppression change: - Drop the TTL gate entirely. A first result arriving after the TTL used to fall through unstamped, so master/fallback routing delivered it — and every later duplicate again, forever: "expired E1 never claimed" and "at-most-once regardless of age" cannot both hold. receipt_from already caps delivery at one per (E1, replier), which is the only guarantee the TTL meaningfully added; late results from long-running tasks are the normal case, not abuse. DELEGATION_RECEIPT_TTL_HOURS is removed with it. - Filter onward delegates against live WorkspaceMember rows. Soft removal keeps both the WorkspaceMember row (status=removed) and the stale ChannelMember row, and mention parsing does not exclude removed members, so "@C" could mint a server-written delegation to an agent that can neither poll nor rejoin — and when the delegator was also gone, the receipt targeted ONLY the dead C. One membership query now covers delegator, replier, and all onward candidates. Tests: late first result delivers once then suppressed (dynamic and master), soft-removed onward dropped without a delegation mark, removed delegator + removed onward → sentinel without router, duplicate with removed onward → sentinel. Backend 37 receipt cases. --- workspace/backend/app/config.py | 6 -- workspace/backend/app/mods/workspace_mod.py | 59 +++++++------ .../backend/tests/test_receipt_routing.py | 83 ++++++++++++++++--- 3 files changed, 101 insertions(+), 47 deletions(-) diff --git a/workspace/backend/app/config.py b/workspace/backend/app/config.py index a3461fc1b..f6b135a71 100644 --- a/workspace/backend/app/config.py +++ b/workspace/backend/app/config.py @@ -78,12 +78,6 @@ class Config: CLOUD_AGENT_MAX_CONTEXT_MESSAGES: int = int(os.environ.get("CLOUD_AGENT_MAX_CONTEXT_MESSAGES", "10")) CLOUD_AGENT_MAX_DEPTH: int = int(os.environ.get("CLOUD_AGENT_MAX_DEPTH", "3")) - # Delegation receipts — how long a delegation message can be referenced by - # a structured receipt (in_reply_to + reply_kind) before it goes stale. - # Product policy, not a technical constant: raise it per deployment when - # agents run tasks longer than a day. - DELEGATION_RECEIPT_TTL_HOURS: float = float(os.environ.get("DELEGATION_RECEIPT_TTL_HOURS", "24")) - # Google OAuth (for "Sign in with Google" Gemini integration) GOOGLE_OAUTH_CLIENT_ID: str = os.environ.get("GOOGLE_OAUTH_CLIENT_ID", "") GOOGLE_OAUTH_CLIENT_SECRET: str = os.environ.get("GOOGLE_OAUTH_CLIENT_SECRET", "") diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index 19ed9893b..e53a2d211 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -15,7 +15,6 @@ import logging import re -import time from datetime import datetime, timezone from typing import List, Optional, Tuple @@ -668,12 +667,13 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> starts with the delegator; ``onward_delegates`` are additional agents the reply @mentions (an onward delegation, dual-routed so the delegator still sees the outcome). Returns ``None`` ONLY when the message is not an - authenticated receipt (or its E1 is past the TTL — see below), in which - case the caller falls through to normal routing. An authenticated receipt - whose delegator is undeliverable (left the channel, removed from the - workspace) or already served returns ``(onward, onward)`` — possibly - empty — so it can never re-enter master/LLM orchestration and be - misrouted back to a stale delegator. + authenticated receipt, in which case the caller falls through to normal + routing. An authenticated receipt whose delegator is undeliverable (left + the channel, removed from the workspace) or already served returns + ``(onward, onward)`` — possibly empty — so it can never re-enter + master/LLM orchestration and be misrouted back to a stale delegator. + Onward delegates are filtered the same way: a removed agent can never + become a deterministic target. Delivery to the delegator is at-most-once per (E1, replier): the first terminal receipt stamps E1 with ``receipt_from`` (under a row lock, so @@ -687,7 +687,6 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> cancelled claim the single terminal slot. If the delegator needs more work done it delegates again, creating a new E1. """ - from app.config import config from app.models import EventRecord source = event.source or "" @@ -756,45 +755,45 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> onward.append(m) seen.add(m) - # Both ends must still be members of this channel — workspace membership - # alone would let a receipt re-target someone who already left the room. - if delegator not in participants or replier not in participants: - return list(onward), onward - - # ... and of the workspace itself. ChannelMember rows are not cleaned up - # when a workspace member is removed (remove_member deletes only the - # WorkspaceMember row; network removal soft-deletes with status - # "removed"), so a stale channel participant could otherwise become a - # receipt target that can no longer poll the workspace. + # Everyone the receipt would target — the delegator AND any onward + # delegates — must be a live workspace member. ChannelMember rows are + # not cleaned up when a workspace member is removed (remove_member + # deletes only the WorkspaceMember row; network removal soft-deletes + # with status "removed") and mention parsing does not exclude removed + # members, so a stale channel participant could otherwise become a + # deterministic target that can no longer poll the workspace. from app.models import WorkspaceMember + names_to_check = list({delegator, replier, *onward}) live_members = { m.agent_name for m in db.execute( select(WorkspaceMember).where( WorkspaceMember.workspace_id == workspace.id, - WorkspaceMember.agent_name.in_([delegator, replier]), + WorkspaceMember.agent_name.in_(names_to_check), ) ).scalars().all() if (m.status or "") != "removed" } + onward = [c for c in onward if c in live_members] + + # Both ends must still be members of this channel and the workspace. + if delegator not in participants or replier not in participants: + return list(onward), onward if delegator not in live_members or replier not in live_members: return list(onward), onward already = e1_meta.get("receipt_from") or [] if replier in already: # Duplicate terminal receipt: the delegator already got their one - # delivery (at-most-once holds regardless of E1 age, so this check - # sits before the TTL). + # delivery. return list(onward), onward - # TTL expiry deliberately falls back to NORMAL routing rather than - # suppression: at this point the delegator is verified alive and present, - # so best-effort orchestration (master star rule, LLM "reports back") - # can still deliver a long-running task's result — suppressing would - # guarantee the loss this feature exists to prevent. The TTL's job is - # only to stop stale E1s from FORCING deterministic routing. - ttl_ms = int(config.DELEGATION_RECEIPT_TTL_HOURS * 3600 * 1000) - if e1.timestamp and (int(time.time() * 1000) - int(e1.timestamp)) > ttl_ms: - return None + # No TTL gate: an authenticated receipt deterministically delivers once + # and claims the (E1, replier) slot regardless of E1's age. receipt_from + # already caps delivery at once, so an age cutoff would add nothing + # except a window where a LATE first result bypasses the claim — its + # duplicates would then be re-delivered forever via fallback routing. + # Long-running tasks finishing days later are the normal case this + # feature exists for, not abuse. # Claim the single terminal receipt — except for needs_input, which is # an intermediate "waiting on you" signal, not the turn's outcome. diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py index bacf09f55..f999fb2dd 100644 --- a/workspace/backend/tests/test_receipt_routing.py +++ b/workspace/backend/tests/test_receipt_routing.py @@ -276,22 +276,27 @@ def test_delegator_left_channel_is_suppressed_not_rerouted(self, db, ws3): assert out.metadata["target_agents"] == ["__no_response__"] mock_client.messages.create.assert_not_called() - def test_expired_delegation_falls_back_to_best_effort_routing(self, db, ws3): - """TTL expiry with a LIVE delegator falls through to normal routing: - the router may still deliver a long task's result best-effort. - Suppression here would guarantee the loss this feature prevents.""" + def test_late_first_result_delivers_once_and_claims(self, db, ws3): + """No TTL gate: a result arriving days after the delegation still + delivers deterministically ONCE and claims the slot — otherwise a + late first result would bypass the claim and its duplicates would be + re-delivered forever via fallback routing.""" ws = ws3["workspace"] - stale = _now_ms() - int(48 * 3600 * 1000) # older than the 24h default TTL + stale = _now_ms() - int(72 * 3600 * 1000) e1 = _delegation_event(db, ws, timestamp=stale) client_patch, mock_client = _with_router("next:agent-a") with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: out = _handle(db, ws, _reply( - "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + "openagents:agent-b", "done at last", in_reply_to=e1.id, reply_kind="result", + )) + out2 = _handle(db, ws, _reply( + "openagents:agent-b", "done at last (retry)", in_reply_to=e1.id, reply_kind="result", )) assert out.metadata["target_agents"] == ["agent-a"] - mock_client.messages.create.assert_called() + assert out2.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() db.refresh(e1) - assert "receipt_from" not in (e1.metadata_ or {}) # expired E1 never claimed + assert e1.metadata_["receipt_from"] == ["agent-b"] def test_duplicate_terminal_receipt_suppressed_without_router(self, db, ws3): """At-most-once: a duplicate must NOT fall back to the LLM router, @@ -401,6 +406,56 @@ def test_undeliverable_receipt_still_honours_onward_mention(self, db, ws3): assert out.metadata["target_agents"] == ["agent-c"] assert out.metadata["delegated_by"] == "agent-b" + def _soft_remove(self, db, ws, name): + from sqlalchemy import select + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == name, + ) + ).scalar_one() + member.status = "removed" + db.flush() + + def test_soft_removed_onward_target_is_dropped(self, db, ws3): + """@C in a receipt must not create a deterministic delegation to a + soft-removed agent — ChannelMember and mention parsing both still + know the stale name.""" + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + self._soft_remove(db, ws, "agent-c") + out = _handle(db, ws, _reply( + "openagents:agent-b", "done — @agent-c could take over", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["agent-a"] + assert "delegated_by" not in out.metadata + assert "delegated_to" not in out.metadata + + def test_removed_delegator_and_soft_removed_onward_gives_sentinel(self, db, ws3): + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + self._soft_remove(db, ws, "agent-a") + self._soft_remove(db, ws, "agent-c") + client_patch, mock_client = _with_router("next:agent-c") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done — @agent-c could take over", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + assert "delegated_by" not in out.metadata + mock_client.messages.create.assert_not_called() + + def test_duplicate_receipt_with_soft_removed_onward_gives_sentinel(self, db, ws3): + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) + self._soft_remove(db, ws, "agent-c") + out = _handle(db, ws, _reply( + "openagents:agent-b", "done again — @agent-c please verify", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + assert "delegated_by" not in out.metadata + def test_consumed_receipt_with_removed_delegator_stays_suppressed(self, db, ws3): ws = ws3["workspace"] e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) @@ -503,16 +558,22 @@ def test_stale_master_receipt_suppressed_not_star_routed(self, db, master_ws, ho )) assert out.metadata["target_agents"] == ["__no_response__"], how - def test_expired_receipt_in_master_mode_star_routes_to_live_master(self, db, master_ws): - """TTL fall-through in master mode: the star rule delivers the late - result to the (verified live) master — best-effort beats loss.""" + def test_late_result_in_master_mode_delivers_once_then_suppressed(self, db, master_ws): + """A late result on an old E1 delivers to the master exactly once; + the duplicate is suppressed instead of riding the star rule back.""" ws = master_ws["workspace"] stale = _now_ms() - int(48 * 3600 * 1000) e1 = _delegation_event(db, ws, timestamp=stale) out = _handle(db, ws, _reply( "openagents:agent-b", "done at last", in_reply_to=e1.id, reply_kind="result", )) + out2 = _handle(db, ws, _reply( + "openagents:agent-b", "done at last (dup)", in_reply_to=e1.id, reply_kind="result", + )) assert out.metadata["target_agents"] == ["agent-a"] + assert out2.metadata["target_agents"] == ["__no_response__"] + db.refresh(e1) + assert e1.metadata_["receipt_from"] == ["agent-b"] def test_plain_sub_agent_message_still_returns_to_master(self, db, master_ws): """Non-receipt agent chatter keeps the existing star behaviour.""" From 3da7f1f68e546f7e10bd338219c2a1f2c4204627 Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 08:28:53 +0000 Subject: [PATCH 6/8] receipts: departed repliers get nothing; bound needs_input replay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A replier that is no longer a live channel + workspace member is fully suppressed — onward included. The events entry point validates only the workspace token/session (_validate_session passes for removed members), so a departed B could otherwise use a stale E1 to mint a server-written delegated_by=B/delegated_to=[C] delegation, and when the delegator was also gone the receipt targeted only C. Only a gone DELEGATOR keeps valid onward mentions alive. - needs_input gets its own replay bound: it deliberately never consumes the terminal slot, so "receipt_from caps delivery at one" did not apply and a single stale E1 was an unlimited deterministic router bypass. Server-owned needs_input_from now counts wake-ups per (E1, replier); up to _NEEDS_INPUT_LIMIT (3) deliver deterministically (several questions in one turn are legitimate — Cline can ask more than once), after which the delegator is suppressed like a duplicate. The terminal slot stays independent, and the field is stripped from inbound metadata like the other server-owned keys. Tests: departed-replier @C across all three removal paths (no routing, no delegation mark, no router call), needs_input limit boundary plus terminal delivery after exhaustion, forged needs_input_from stripped. 38 receipt cases collected. --- workspace/backend/app/mods/workspace_mod.py | 54 ++++++++++--- .../backend/tests/test_receipt_routing.py | 75 +++++++++++++++++++ 2 files changed, 119 insertions(+), 10 deletions(-) diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index e53a2d211..0ae2dffe9 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -651,7 +651,17 @@ def _master_targets(event, channel, mentions: List[str]) -> List[str]: # client, so an agent could submit a forged delegation chain; these keys are # stripped from every inbound message and only written back by the routing # logic below. -_SERVER_OWNED_METADATA = ("delegated_by", "delegated_to", "receipt_from") +_SERVER_OWNED_METADATA = ( + "delegated_by", "delegated_to", "receipt_from", "needs_input_from", +) + +# How many deterministic needs_input wake-ups one delegation event may produce +# per replier. needs_input is non-consuming (the real result must still be +# deliverable afterwards), so without a bound a single stale E1 would be an +# unlimited router bypass. Multiple questions within one turn are legitimate +# (e.g. Cline can surface several asks); past the cap the delegator is +# suppressed like a duplicate receipt. +_NEEDS_INPUT_LIMIT = 3 def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> Optional[Tuple[List[str], List[str]]]: @@ -684,8 +694,14 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> is still honoured. ``needs_input`` receipts are non-consuming: an agent may surface a question mid-turn and still owe the real result (e.g. Cline's ask event followed by the final text), so only result / error / - cancelled claim the single terminal slot. If the delegator needs more - work done it delegates again, creating a new E1. + cancelled claim the single terminal slot. needs_input is still bounded + (``_NEEDS_INPUT_LIMIT`` deterministic wake-ups per (E1, replier), tracked + in server-owned ``needs_input_from``) so a stale E1 cannot become an + unlimited router bypass. A replier that is itself no longer a live + channel + workspace member gets nothing at all — not even onward — since + the message entry point does not verify membership and a departed agent + must not mint new delegations. If the delegator needs more work done it + delegates again, creating a new E1. """ from app.models import EventRecord @@ -775,10 +791,17 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> } onward = [c for c in onward if c in live_members] - # Both ends must still be members of this channel and the workspace. - if delegator not in participants or replier not in participants: - return list(onward), onward - if delegator not in live_members or replier not in live_members: + # A replier that is no longer a live channel + workspace member must not + # be able to mint new delegations from a stale E1: the message entry + # point validates only the workspace token/session, not membership + # (_validate_session passes for removed members). Suppress EVERYTHING, + # onward included. + if replier not in participants or replier not in live_members: + return [], [] + + # A gone delegator only loses their own delivery — the (valid, live) + # onward delegation in the same reply is still honoured. + if delegator not in participants or delegator not in live_members: return list(onward), onward already = e1_meta.get("receipt_from") or [] @@ -795,9 +818,20 @@ def _receipt_route(event: Event, channel, mentions: List[str], db, workspace) -> # Long-running tasks finishing days later are the normal case this # feature exists for, not abuse. - # Claim the single terminal receipt — except for needs_input, which is - # an intermediate "waiting on you" signal, not the turn's outcome. - if meta.get("reply_kind") != "needs_input": + if meta.get("reply_kind") == "needs_input": + # Non-consuming (the real result must still be deliverable), but + # bounded — see _NEEDS_INPUT_LIMIT. + asked = dict(e1_meta.get("needs_input_from") or {}) + count = int(asked.get(replier, 0) or 0) + if count >= _NEEDS_INPUT_LIMIT: + return list(onward), onward + asked[replier] = count + 1 + stamped = dict(e1_meta) + stamped["needs_input_from"] = asked + e1.metadata_ = stamped + db.flush() + else: + # Claim the single terminal receipt. stamped = dict(e1_meta) stamped["receipt_from"] = already + [replier] e1.metadata_ = stamped diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py index f999fb2dd..9df263618 100644 --- a/workspace/backend/tests/test_receipt_routing.py +++ b/workspace/backend/tests/test_receipt_routing.py @@ -164,6 +164,35 @@ def test_mention_of_delegator_is_not_reverse_delegation(self, db, ws3): assert "delegated_by" not in out.metadata assert "delegated_to" not in out.metadata + def test_needs_input_replay_is_bounded(self, db, ws3): + """needs_input never consumes the terminal slot, so it gets its own + bound: _NEEDS_INPUT_LIMIT deterministic wake-ups per (E1, replier), + after which the delegator is suppressed like a duplicate. The real + terminal result afterwards still delivers.""" + from app.mods.workspace_mod import _NEEDS_INPUT_LIMIT + ws, e1 = ws3["workspace"], _delegation_event(db, ws3["workspace"]) + for i in range(_NEEDS_INPUT_LIMIT): + out = _handle(db, ws, _reply( + "openagents:agent-b", f"question {i}?", in_reply_to=e1.id, + reply_kind="needs_input", + )) + assert out.metadata["target_agents"] == ["agent-a"], i + client_patch, mock_client = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "one more question?", in_reply_to=e1.id, + reply_kind="needs_input", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() + db.refresh(e1) + assert e1.metadata_["needs_input_from"] == {"agent-b": _NEEDS_INPUT_LIMIT} + # Terminal slot is independent of the needs_input budget. + out2 = _handle(db, ws, _reply( + "openagents:agent-b", "done", in_reply_to=e1.id, reply_kind="result", + )) + assert out2.metadata["target_agents"] == ["agent-a"] + def test_onward_mention_dual_routes(self, db, ws3): """Receipt that @mentions a third agent goes to both A and C, and the onward hop is marked as a new delegation by B.""" @@ -210,11 +239,13 @@ def test_forged_server_owned_metadata_is_stripped(self, db, ws3): "delegated_by": "agent-b", "delegated_to": ["agent-a"], "receipt_from": ["agent-c"], + "needs_input_from": {"agent-b": 99}, }, )) assert "delegated_by" not in out.metadata assert "delegated_to" not in out.metadata assert "receipt_from" not in out.metadata + assert "needs_input_from" not in out.metadata def test_e1_missing(self, db, ws3): ws = ws3["workspace"] @@ -456,6 +487,50 @@ def test_duplicate_receipt_with_soft_removed_onward_gives_sentinel(self, db, ws3 assert out.metadata["target_agents"] == ["__no_response__"] assert "delegated_by" not in out.metadata + def _remove_replier(self, db, ws3, how): + from sqlalchemy import select + ws, ch = ws3["workspace"], ws3["channel"] + if how == "left_channel": + member = db.execute( + select(ChannelMember).where( + ChannelMember.channel_id == ch.id, + ChannelMember.agent_name == "agent-b", + ) + ).scalar_one() + db.delete(member) + else: + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-b", + ) + ).scalar_one() + if how == "hard": + db.delete(member) + else: + member.status = "removed" + db.flush() + db.refresh(ch) + + @pytest.mark.parametrize("how", ["left_channel", "hard", "soft"]) + def test_departed_replier_cannot_mint_onward_delegation(self, db, ws3, how): + """The message entry point validates only token/session, so a removed + B can still post against an old E1 — it must get nothing routed, and + especially must not create a delegated_by=B onward delegation.""" + ws = ws3["workspace"] + e1 = _delegation_event(db, ws) + self._remove_replier(db, ws3, how) + client_patch, mock_client = _with_router("next:agent-c") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "done — @agent-c take over", + in_reply_to=e1.id, reply_kind="result", + )) + assert out.metadata["target_agents"] == ["__no_response__"], how + assert "delegated_by" not in out.metadata, how + assert "delegated_to" not in out.metadata, how + mock_client.messages.create.assert_not_called() + def test_consumed_receipt_with_removed_delegator_stays_suppressed(self, db, ws3): ws = ws3["workspace"] e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) From d1aa33a5a93205ecbe1717bb00a446d867001d20 Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 09:56:35 +0000 Subject: [PATCH 7/8] gate agent routing on live membership, not just receipts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The departed-replier check lived only inside _receipt_route, so a removed B could drop in_reply_to/reply_kind and send a plain "@C do X" — ordinary routing then still delivered it and wrote a server-marked delegated_by=B/delegated_to=[C] (_validate_session passes for missing and soft-removed members). Two general gates now sit in front of ALL agent-chat routing: - sender gate: an agent message whose sender is not a live WorkspaceMember AND a current ChannelMember goes straight to the no-response sentinel — no receipt handling, no router, no marks. Routine channels register their owner as a ChannelMember on creation, so system job queues are unaffected. - target gate: agent-sourced routing results are filtered to live current participants — the mention fallback can name a live agent outside the channel, and router/master candidate lists can include the stale ChannelMember row of a removed agent. Human messages keep the wider net (their targets are auto-added to the channel). Mention parsing now excludes soft-removed members up front (live_agents replaces known_agents), so a removed name is no candidate anywhere — routing, delegation marks, or receipt onward. Tests: departed sender's plain @C across all three removal paths (no routing, no marks, no router call), live sender's plain @C to a soft-removed target dropped even when the router picks the stale name. 42 receipt cases, router suite unchanged. --- workspace/backend/app/mods/workspace_mod.py | 38 +++++++++++++++++-- .../backend/tests/test_receipt_routing.py | 32 ++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index 0ae2dffe9..3fcbdb26d 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -1279,15 +1279,18 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional if message_type in ("thinking", "status", "todos"): return event - # Parse @mentions from message content (used for human message routing) - known_agents = [ + # Parse @mentions from message content (used for human message routing). + # Soft-removed members are excluded up front: a removed agent must not be + # a mention candidate anywhere — not in routing, not in delegation marks. + live_agents = { m.agent_name for m in db.execute( select(WorkspaceMember).where( WorkspaceMember.workspace_id == workspace.id, ) ).scalars().all() - ] - mentions = _extract_mentions(content, known_agents) + if (m.status or "") != "removed" + } + mentions = _extract_mentions(content, sorted(live_agents)) # Resolve channel (needed for both agent and human message routing) channel = None @@ -1326,6 +1329,25 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional p for p in (channel.participants or []) if p.agent_name != "__no_response__" ] + participant_names = {p.agent_name for p in real_participants} + + # ── Sender gate for agent messages ── + # The events entry point validates only the workspace token/session + # (_validate_session passes for missing and soft-removed members), so a + # departed agent can still post. Its messages must not enter routing at + # all — via the receipt path OR a plain "@C do X" — or it could keep + # minting server-marked delegations after removal. Live workspace + # membership AND current channel membership are both required (routine + # channels register their owner as a ChannelMember on creation). + if event.source.startswith("openagents:"): + _sender = event.source[len("openagents:"):] + if _sender not in live_agents or _sender not in participant_names: + logger.info( + "workspace_mod: dropped message from departed agent %s in %s", + _sender, channel.name, + ) + event.metadata["target_agents"] = ["__no_response__"] + return event # ── Structured delegation receipt: deterministic return to delegator ── # Checked before the participant-count branch so a receipt still lands @@ -1373,6 +1395,14 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional else: targets = _fallback_targets(event, channel, mentions, online_names) + # Agent-sourced routing may only target live, current participants: the + # mention fallback can name a live agent outside the channel, and the + # router/master candidate lists can include the stale ChannelMember row + # of a removed agent. Human messages keep the wider net — agents a human + # targets are auto-added to the channel below. + if event.source.startswith("openagents:") and targets: + targets = [t for t in targets if t in participant_names and t in live_agents] + # ── Mark explicit delegations (server-owned metadata) ── # Only agent messages whose routed targets came from the sender's own # @mentions count as delegations; router-inferred hops (e.g. "report to diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py index 9df263618..1f51a2264 100644 --- a/workspace/backend/tests/test_receipt_routing.py +++ b/workspace/backend/tests/test_receipt_routing.py @@ -512,6 +512,38 @@ def _remove_replier(self, db, ws3, how): db.flush() db.refresh(ch) + @pytest.mark.parametrize("how", ["left_channel", "hard", "soft"]) + def test_departed_sender_plain_message_is_dropped(self, db, ws3, how): + """The sender gate must also cover PLAIN messages: a departed B could + otherwise skip the receipt fields entirely and still mint a + delegated_by=B mark via ordinary routing.""" + ws = ws3["workspace"] + self._remove_replier(db, ws3, how) + client_patch, mock_client = _with_router("next:agent-c") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-b", "@agent-c please go do X", + )) + assert out.metadata["target_agents"] == ["__no_response__"], how + assert "delegated_by" not in out.metadata, how + assert "delegated_to" not in out.metadata, how + mock_client.messages.create.assert_not_called() + + def test_plain_delegation_to_soft_removed_target_is_dropped(self, db, ws3): + """A live agent's ordinary "@C do X" must not route to or mark a + soft-removed C — mention parsing and the router candidate list both + still know the stale name.""" + ws = ws3["workspace"] + self._soft_remove(db, ws, "agent-c") + client_patch, _ = _with_router("next:agent-c") # router picks the stale name + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply( + "openagents:agent-a", "@agent-c please go do X", + )) + assert out.metadata["target_agents"] == ["__no_response__"] + assert "delegated_by" not in out.metadata + assert "delegated_to" not in out.metadata + @pytest.mark.parametrize("how", ["left_channel", "hard", "soft"]) def test_departed_replier_cannot_mint_onward_delegation(self, db, ws3, how): """The message entry point validates only token/session, so a removed From 0039d990c39c08d365f6bc26a53bd296ce3d7f8d Mon Sep 17 00:00:00 2001 From: QuanCheng <915158214@qq.com> Date: Sun, 2 Aug 2026 10:19:01 +0000 Subject: [PATCH 8/8] routing never targets removed agents; departed senders are rejected outright MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on the membership gates: - The live-member filter now applies to EVERY routing result, not just agent-sourced ones. channel.master_agent and ChannelMember rows both survive member removal, so a human message in master mode was handed straight to the deleted master (and the dynamic router's all-offline fallback re-admitted removed names). Master dispatch falls back when the recorded master is not a live member, _fallback_targets skips non-live masters/participants, and the router's candidate list drops removed members before the model ever sees them (its human safety net fallback is live-filtered too). Only the "must be a current channel participant" restriction stays agent-only — human-targeted agents are auto-added to the channel. - The departed-sender gate raises EventRejected ("agent_removed" / "channel_membership_required") instead of routing to the sentinel. A sentineled event still persisted and pushed to humans, so a zombie daemon of a removed agent could keep writing visible messages and firing notifications. Rejection keeps the event out of persistence, push fan-out, SSE, and cloud-agent invocation entirely; the reason string matches the join-path rejection so clients handle one code. Known trust boundary (for the PR): these gates trust event.source. The workspace token is shared and missing session_ids pass as legacy, so this stops honest-but-stale daemons, not a token-holding client that impersonates a live agent. Tests: departed sender/replier rejection with exact reasons across all three removal paths, human message with deleted master falls back to a live participant, router picking a removed name falls back for humans. 44 receipt cases; full backend 544 passed with the same pre-existing environment failures. --- workspace/backend/app/mods/workspace_mod.py | 98 ++++++++++++++----- .../backend/tests/test_receipt_routing.py | 82 +++++++++++----- 2 files changed, 130 insertions(+), 50 deletions(-) diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index 3fcbdb26d..38d73adc7 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -576,25 +576,39 @@ def _online_participant_names(db, workspace, channel) -> set: return set() -def _fallback_targets(event, channel, mentions: List[str], online_names: set = None) -> List[str]: +def _fallback_targets(event, channel, mentions: List[str], online_names: set = None, + live: set = None) -> List[str]: """Determine target agents when LLM router is unavailable. Priority: explicit @mentions → master (for human/member msgs) → online participant → any participant. When ``online_names`` is provided, an online participant is chosen over an offline one so messages aren't stranded on a dead agent. An explicit @mention is always honored as-is (the user chose it). + + When ``live`` is provided it is the set of non-removed workspace members: + the master and participant branches skip anyone outside it, because + ``channel.master_agent`` and ChannelMember rows both survive member + removal — without the filter a human message would be routed straight to + a deleted agent and never get handled. """ if mentions: - return [mentions[0]] - if channel.master_agent: + picked = [m for m in mentions if live is None or m in live] + if picked: + return [picked[0]] + master = channel.master_agent + if master and (live is None or master in live): if event.source.startswith("openagents:"): sender = event.source[len("openagents:"):] # Master's own messages: no self-trigger - if sender == channel.master_agent: + if sender == master: return [] - return [channel.master_agent] - # No master — prefer an online participant, else the first participant. - participants = [p.agent_name for p in (channel.participants or [])] + return [master] + # No (live) master — prefer an online participant, else the first + # live participant. + participants = [ + p.agent_name for p in (channel.participants or []) + if live is None or p.agent_name in live + ] if online_names: online_first = [p for p in participants if p in online_names] if online_first: @@ -1009,6 +1023,15 @@ async def _route_with_llm( ) ).scalars().all() } + # Removed members are never routing candidates: hard-deleted ones have no + # WorkspaceMember row at all, soft-deleted ones carry status="removed", + # and both can leave a stale ChannelMember row behind. Filtering here + # (rather than after the LLM picks) keeps the router from selecting a + # name that would then just be dropped. + participant_names = [ + n for n in participant_names + if members.get(n) is not None and (members[n].status or "") != "removed" + ] # Only offer ONLINE participants to the router when any are online — an # offline agent (dead daemon) can't reply, so routing to it by # conversational continuity just strands the message. If none are online, @@ -1128,7 +1151,9 @@ async def _route_with_llm( # router can silently drop a legitimate follow-up question like # "how about Julia?" after a previous "final answer" message. if (new_event.source or "").startswith("human:"): - fallback = _fallback_targets(new_event, channel, [], online_set) + fallback = _fallback_targets( + new_event, channel, [], online_set, live=set(participant_names), + ) if fallback: logger.info( "LLM router returned stop/invalid for human message — " @@ -1341,13 +1366,23 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional # channels register their owner as a ChannelMember on creation). if event.source.startswith("openagents:"): _sender = event.source[len("openagents:"):] - if _sender not in live_agents or _sender not in participant_names: + if _sender not in live_agents: + # Rejecting (rather than routing to the sentinel) keeps the + # message out of persistence and push fan-out entirely — a + # zombie daemon of a removed agent must not keep writing + # visible messages or notifying humans. Same reason string as + # the join-path rejection so clients handle one code. logger.info( - "workspace_mod: dropped message from departed agent %s in %s", + "workspace_mod: rejected message from removed agent %s in %s", _sender, channel.name, ) - event.metadata["target_agents"] = ["__no_response__"] - return event + raise EventRejected("workspace_mod", "agent_removed") + if _sender not in participant_names: + logger.info( + "workspace_mod: rejected message from non-member %s in %s", + _sender, channel.name, + ) + raise EventRejected("workspace_mod", "channel_membership_required") # ── Structured delegation receipt: deterministic return to delegator ── # Checked before the participant-count branch so a receipt still lands @@ -1372,13 +1407,16 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional mode = (getattr(channel, "orchestration_mode", None) or "dynamic").lower() if mode == "master": - # Deterministic star topology — no LLM. If the channel somehow - # has no master, fall back to the generic mention/online logic - # so messages aren't stranded. - if channel.master_agent: + # Deterministic star topology — no LLM. If the channel has no + # master, or the recorded master is no longer a live workspace + # member (channel.master_agent survives member removal), fall + # back to the generic mention/online logic so messages aren't + # stranded on a deleted hub. + if channel.master_agent and channel.master_agent in live_agents: targets = _master_targets(event, channel, mentions) else: - targets = _fallback_targets(event, channel, mentions, online_names) + targets = _fallback_targets(event, channel, mentions, online_names, + live=live_agents) elif mode == "workflow" and config.ROUTER_LLM_ENABLED and _get_router_api_key(): # LLM router steered by the user's natural-language plan. targets = await _route_with_llm( @@ -1390,18 +1428,24 @@ async def _handle_message_posted(event: Event, ctx: PipelineContext) -> Optional targets = await _route_with_llm(channel, event, db, workspace) else: # LLM router not available — fallback to mention or master. - targets = _fallback_targets(event, channel, mentions, online_names) + targets = _fallback_targets(event, channel, mentions, online_names, + live=live_agents) # ── Single-agent channel ──────────────────────────────────────── else: - targets = _fallback_targets(event, channel, mentions, online_names) - - # Agent-sourced routing may only target live, current participants: the - # mention fallback can name a live agent outside the channel, and the - # router/master candidate lists can include the stale ChannelMember row - # of a removed agent. Human messages keep the wider net — agents a human - # targets are auto-added to the channel below. - if event.source.startswith("openagents:") and targets: - targets = [t for t in targets if t in participant_names and t in live_agents] + targets = _fallback_targets(event, channel, mentions, online_names, + live=live_agents) + + # No routing result may target a removed agent, whatever the source — + # channel.master_agent and ChannelMember rows survive member removal, so + # master/router/fallback candidates can all carry stale names and a human + # message would otherwise be handed to an agent that can no longer poll. + # Agent-sourced messages are additionally confined to current channel + # participants; humans keep the wider net (their targets are auto-added + # to the channel below). + if targets: + targets = [t for t in targets if t in live_agents] + if event.source.startswith("openagents:"): + targets = [t for t in targets if t in participant_names] # ── Mark explicit delegations (server-owned metadata) ── # Only agent messages whose routed targets came from the sender's own diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py index 1f51a2264..e1faa071b 100644 --- a/workspace/backend/tests/test_receipt_routing.py +++ b/workspace/backend/tests/test_receipt_routing.py @@ -20,7 +20,7 @@ ) from app.mods.workspace_mod import _handle_message_posted from openagents.core.onm_events import Event -from openagents.core.onm_mods import PipelineContext +from openagents.core.onm_mods import EventRejected, PipelineContext def _run(coro): @@ -512,21 +512,24 @@ def _remove_replier(self, db, ws3, how): db.flush() db.refresh(ch) - @pytest.mark.parametrize("how", ["left_channel", "hard", "soft"]) - def test_departed_sender_plain_message_is_dropped(self, db, ws3, how): - """The sender gate must also cover PLAIN messages: a departed B could - otherwise skip the receipt fields entirely and still mint a - delegated_by=B mark via ordinary routing.""" + @pytest.mark.parametrize("how,reason", [ + ("left_channel", "channel_membership_required"), + ("hard", "agent_removed"), + ("soft", "agent_removed"), + ]) + def test_departed_sender_plain_message_is_rejected(self, db, ws3, how, reason): + """The sender gate must also cover PLAIN messages, and it must REJECT + rather than sentinel: a sentineled event still persists and pushes to + humans, so a zombie daemon could keep spamming the channel.""" ws = ws3["workspace"] self._remove_replier(db, ws3, how) client_patch, mock_client = _with_router("next:agent-c") with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: - out = _handle(db, ws, _reply( - "openagents:agent-b", "@agent-c please go do X", - )) - assert out.metadata["target_agents"] == ["__no_response__"], how - assert "delegated_by" not in out.metadata, how - assert "delegated_to" not in out.metadata, how + with pytest.raises(EventRejected) as exc: + _handle(db, ws, _reply( + "openagents:agent-b", "@agent-c please go do X", + )) + assert exc.value.reason == reason, how mock_client.messages.create.assert_not_called() def test_plain_delegation_to_soft_removed_target_is_dropped(self, db, ws3): @@ -544,25 +547,58 @@ def test_plain_delegation_to_soft_removed_target_is_dropped(self, db, ws3): assert "delegated_by" not in out.metadata assert "delegated_to" not in out.metadata - @pytest.mark.parametrize("how", ["left_channel", "hard", "soft"]) - def test_departed_replier_cannot_mint_onward_delegation(self, db, ws3, how): + @pytest.mark.parametrize("how,reason", [ + ("left_channel", "channel_membership_required"), + ("hard", "agent_removed"), + ("soft", "agent_removed"), + ]) + def test_departed_replier_receipt_is_rejected(self, db, ws3, how, reason): """The message entry point validates only token/session, so a removed - B can still post against an old E1 — it must get nothing routed, and - especially must not create a delegated_by=B onward delegation.""" + B can still post against an old E1 — the sender gate rejects it before + any routing or delegation marking.""" ws = ws3["workspace"] e1 = _delegation_event(db, ws) self._remove_replier(db, ws3, how) client_patch, mock_client = _with_router("next:agent-c") with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: - out = _handle(db, ws, _reply( - "openagents:agent-b", "done — @agent-c take over", - in_reply_to=e1.id, reply_kind="result", - )) - assert out.metadata["target_agents"] == ["__no_response__"], how - assert "delegated_by" not in out.metadata, how - assert "delegated_to" not in out.metadata, how + with pytest.raises(EventRejected) as exc: + _handle(db, ws, _reply( + "openagents:agent-b", "done — @agent-c take over", + in_reply_to=e1.id, reply_kind="result", + )) + assert exc.value.reason == reason, how mock_client.messages.create.assert_not_called() + def test_human_message_not_routed_to_deleted_master(self, db, ws3): + """Stale channel.master_agent + ChannelMember must not receive a + human's message — routing falls back to a live participant.""" + ws, ch = ws3["workspace"], ws3["channel"] + ch.master_agent = "agent-a" + ch.orchestration_mode = "master" + db.flush() + from sqlalchemy import select + member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == ws.id, + WorkspaceMember.agent_name == "agent-a", + ) + ).scalar_one() + db.delete(member) # hard delete; ChannelMember + master_agent remain + db.flush() + db.refresh(ch) + out = _handle(db, ws, _reply("human:user", "please handle this")) + assert out.metadata["target_agents"] == ["agent-b"] + + def test_router_pick_of_removed_agent_falls_back_for_human(self, db, ws3): + """Router candidates exclude removed members; if the model still emits + a stale name, the human safety net routes to a live participant.""" + ws = ws3["workspace"] + self._soft_remove(db, ws, "agent-a") + client_patch, _ = _with_router("next:agent-a") + with ROUTER_PATCHES[0], ROUTER_PATCHES[1], client_patch: + out = _handle(db, ws, _reply("human:user", "please handle this")) + assert out.metadata["target_agents"] == ["agent-b"] + def test_consumed_receipt_with_removed_delegator_stays_suppressed(self, db, ws3): ws = ws3["workspace"] e1 = _delegation_event(db, ws, receipt_from=("agent-b",))