diff --git a/packages/agent-connector/src/adapters/aider.js b/packages/agent-connector/src/adapters/aider.js index 87339c8e6..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; } @@ -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..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)}`, ); @@ -288,9 +289,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..7a136657d 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,77 @@ 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. + // + // 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 = {}; + const id = triggerMsg && triggerMsg.messageId; + const sender = (triggerMsg && triggerMsg.senderName) || ''; + 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; + } + + 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..a13de356f 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; @@ -1292,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; } @@ -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..e3b165a3f 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 {} } } @@ -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; @@ -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..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; } @@ -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..a4c68b948 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 {} } } @@ -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; } @@ -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); } @@ -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 12bfe91a1..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; } @@ -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); } @@ -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 dfe3295c6..3651ee3f3 100644 --- a/packages/agent-connector/src/adapters/goose.js +++ b/packages/agent-connector/src/adapters/goose.js @@ -364,12 +364,13 @@ 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.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, ' @@ -382,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); } @@ -441,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); @@ -462,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(); @@ -485,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; } @@ -515,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.', @@ -551,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)); }); @@ -582,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; } @@ -592,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/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..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.` ); @@ -113,13 +114,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..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)}`, ); @@ -393,15 +394,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..5ef538e16 --- /dev/null +++ b/packages/agent-connector/test/terminal-replies.test.js @@ -0,0 +1,192 @@ +'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; +} + +// 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 () => { + 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('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, undefined); + assert.equal(meta.in_reply_to, undefined); + }); + + 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); + 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 () => { + 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('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(); + 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/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index dc1de3d41..38d73adc7 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -16,7 +16,7 @@ import logging import re from datetime import datetime, timezone -from typing import List, Optional +from typing import List, Optional, Tuple from sqlalchemy import select @@ -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: @@ -638,6 +652,208 @@ 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", "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]]]: + """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`` 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`` ONLY when the message is not an + 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 + 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. 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 + + 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 + + # 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 + # 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 + + # 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) + + # 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_(names_to_check), + ) + ).scalars().all() + if (m.status or "") != "removed" + } + onward = [c for c in onward if c 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 [] + if replier in already: + # Duplicate terminal receipt: the delegator already got their one + # delivery. + return list(onward), onward + + # 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. + + 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 + db.flush() + + 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 \ @@ -807,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, @@ -926,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 — " @@ -1047,6 +1274,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 @@ -1070,15 +1304,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 @@ -1117,18 +1354,69 @@ 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: + 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: + # 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: rejected message from removed agent %s in %s", + _sender, channel.name, + ) + 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 + # 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() 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( @@ -1140,10 +1428,42 @@ 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) + 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 + # @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. # diff --git a/workspace/backend/tests/test_receipt_routing.py b/workspace/backend/tests/test_receipt_routing.py new file mode 100644 index 000000000..e1faa071b --- /dev/null +++ b/workspace/backend/tests/test_receipt_routing.py @@ -0,0 +1,762 @@ +# -*- 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 EventRejected, 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_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.""" + 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"], + "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"] + 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_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 + 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) + 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_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(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 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"] + assert out2.metadata["target_agents"] == ["__no_response__"] + mock_client.messages.create.assert_not_called() + db.refresh(e1) + 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, + which could re-select the delegator — it is suppressed outright.""" + ws = ws3["workspace"] + e1 = _delegation_event(db, ws, receipt_from=("agent-b",)) + 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() + 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"] + 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() + 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 _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 _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,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: + 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): + """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,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 — 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: + 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",)) + 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 + 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 + + 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_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.""" + 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): + """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