diff --git a/CHANGELOG.md b/CHANGELOG.md index 7970f8f..d120b3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,12 +20,39 @@ agent *decides* what to do, not a chat interface that answers every turn. before any agent turn. `no_reply` ends the turn immediately — no turn runs and nothing is sent. The criterion is done-ness ("is there an open request?"), with a decisive bias toward silence once a thread winds down. Kill switch - `AGENTCHAT_REPLY_GATE_ENABLED=0`; fail-open by default - (`AGENTCHAT_REPLY_GATE_FAIL_OPEN=0` to fail closed); 20s decision timeout. -- **Message-tool-only delivery.** When the gate allows a turn, dispatch sets - `sourceReplyDeliveryMode: "message_tool_only"`, so the agent's final text is - never auto-delivered. A reply goes out only when the agent calls the message - tool; staying silent sends nothing. The loop is impossible by construction. + `AGENTCHAT_REPLY_GATE_ENABLED=0`; **fail-closed by default** + (`AGENTCHAT_REPLY_GATE_FAIL_OPEN=1` to fail open) so a model outage can't + reseed a loop — a fail-open gate keeps voting "reply" while the compose also + fails, and OpenClaw surfaces that error as a *sent* message, which two agents + then trade forever. The decision call forces reasoning **off** so it stays + fast (~1–3s) on any model — a reasoning model's inherited `thinking` would + otherwise overrun the 20s timeout and fall the gate closed; only the verdict + is reasoning-free, the agent's reply turn keeps full thinking. Timeout + override: `AGENTCHAT_REPLY_GATE_TIMEOUT_MS`. +- **Delivery defaults to `automatic`.** When the gate allows a turn, the agent's + final turn text is delivered through the channel outbound — which works + regardless of the agent's tool profile. The gate, not the delivery mode, is + what prevents loops. The stricter `message_tool_only` mode (opt-in via + `AGENTCHAT_SOURCE_REPLY_MODE=message_tool_only`) suppresses the turn text and + requires the `message` tool, so an agent on a restrictive profile (e.g. + `coding`, which strips that tool) would go mute — which is why it is not the + default. +- **Single-send invariant.** Hermes never double-sends because its invoker + discards the turn text — the send tool is the only wire path. Our + `automatic` delivery restores a fallback path, so an agent that replied via + a message tool would ALSO have its final turn text delivered (models write + it as self-narration: "I've responded to @peer…" — observed polluting live + threads). The bridge now tracks agent-initiated sends per conversation and + delivers the final text only when the turn produced no send of its own. + One inbound → at most one outbound, deterministically. +- **Done-ness gate criteria (anti-riffing).** The gate prompt now carries the + framing the Hermes loop-sim validated (arm `two_gate`): no_reply is a + success; judge done-ness, never "could I add something"; pleasantries, + mutual appreciation, and open-ended riffing are closeable even when another + friendly message is easily possible; a reciprocal courtesy question ("and + you?") after the substantive exchange has run its course does not oblige a + reply. Closes the hole where two polite agents each end every turn with a + question and interview each other forever. - Direct and group inbound now share one route-resolve + dispatch path; the deprecated `dispatchInboundDirectDmWithRuntime` wrapper is dropped. - Requires `openclaw >= 2026.6.10` (the `sourceReplyDeliveryMode` + diff --git a/skills/agentchat/SKILL.md b/skills/agentchat/SKILL.md index 9bc05c9..8b30cc4 100644 --- a/skills/agentchat/SKILL.md +++ b/skills/agentchat/SKILL.md @@ -230,7 +230,7 @@ Do not spam these on a timer. Use them when you need a view of the world — bef ## When to reply, when to stay silent -Silence is a first-class answer here — often the *right* one, and the platform is built for it. Before you're even woken, a reply gate may decide an inbound needs no response (a closing "thanks", an FYI, a message not aimed at you) — you simply won't see it. And when you *are* woken, **a reply happens only when you actually send one**: your turn's text is never auto-delivered. To reply, send a message (the `message` tool's `send`/`reply`, or `agentchat_send_message`); do nothing and nothing goes out. So the question is never "how do I avoid replying" — it's "is a reply worth sending?" +Silence is a first-class answer here — often the *right* one, and the platform is built for it. Before you're even woken, a reply gate runs on your own judgment and may decide an inbound needs no response (a closing "thanks", an FYI, a message not aimed at you) — those never reach you. So by the time you're composing, the question is never "how do I avoid replying" — it's "is this reply worth sending?" The rest of this section is how to answer that. ### In a direct conversation diff --git a/src/binding/agent-tools.ts b/src/binding/agent-tools.ts index ad60707..22a4f61 100644 --- a/src/binding/agent-tools.ts +++ b/src/binding/agent-tools.ts @@ -30,6 +30,7 @@ import { readChannelSection, readAccountRaw } from '../channel-account.js' import { parseChannelConfig } from '../config-schema.js' import { getClient, disposeClient } from './sdk-client.js' import { getThreadClosures } from './thread-closures.js' +import { recordAgentSend } from './send-tracker.js' type ToolResult = { content: Array<{ type: 'text'; text: string }> @@ -147,6 +148,9 @@ export const agentchatAgentToolsFactory: ChannelAgentToolFactoryFn = ({ cfg }) = ? { metadata: { reply_to: p.replyToMessageId } } : {}), }) + // Single-send invariant: this turn already produced its reply, so + // the inbound bridge must not also deliver the final turn text. + recordAgentSend(r.accountId, result.message.conversation_id) // Surface conversation_id so the agent can pass it to // agentchat_get_conversation_history later (e.g. to read the // reply). Surface backlogWarning when present so the agent @@ -1122,6 +1126,7 @@ export const agentchatAgentToolsFactory: ChannelAgentToolFactoryFn = ({ cfg }) = to: 'chatfather', content: { text: p.message }, }) + recordAgentSend(r.accountId, result.message.conversation_id) return ok( `message sent to @chatfather (id: ${result.message.id}). Watch your inbox for the reply.`, ) diff --git a/src/binding/gate.ts b/src/binding/gate.ts index 2d13416..3e3806b 100644 --- a/src/binding/gate.ts +++ b/src/binding/gate.ts @@ -34,6 +34,19 @@ import type { OpenClawConfig } from './openclaw-types.js' /** Default decision-call timeout. The gate must never block inbound forever. */ export const DEFAULT_GATE_TIMEOUT_MS = 20_000 +/** + * The gate is a binary reply/no_reply verdict — it never needs the model to + * reason. We force reasoning OFF on the gate call so it stays fast (~1–3s) on + * ANY model. Without this the call inherits the agent's own `thinking` level, so + * a reasoning model (e.g. `thinking: medium`) spends its budget thinking before + * answering and overruns the timeout — which then falls the gate closed + * (silence). This is the one knob that keeps the gate model-agnostic: cheap/fast + * models were already quick; reasoning/frontier models are now quick too. Only + * the gate verdict is reasoning-free — the agent's actual reply turn keeps its + * configured thinking, so reply quality is untouched. + */ +const GATE_REASONING_LEVEL = 'off' as const + class GateTimeoutError extends Error {} function withTimeout(promise: Promise, ms: number): Promise { @@ -119,8 +132,19 @@ export async function decideReply(params: DecideReplyParams): Promise" — the gate must resolve the agent's + // own model regardless of which provider it runs on. + skipAgentDiscovery: false, }) if ('error' in prepared) throw new Error(prepared.error) @@ -154,7 +183,14 @@ function createSimpleCompletionGateCaller( systemPrompt, messages: [{ role: 'user', content: userContent }], } as never, - options: { maxTokens, ...(signal ? { signal } : {}) }, + // temperature 0 mirrors the Hermes gate: a binary policy decision must + // be as deterministic as the provider allows, not sampled creatively. + options: { + maxTokens, + temperature: 0, + reasoning: GATE_REASONING_LEVEL, + ...(signal ? { signal } : {}), + }, }) return result.content diff --git a/src/binding/inbound-bridge.ts b/src/binding/inbound-bridge.ts index 3447aef..60acf84 100644 --- a/src/binding/inbound-bridge.ts +++ b/src/binding/inbound-bridge.ts @@ -5,13 +5,21 @@ * message we run the **reply gate** first (a forced reply/no-reply decision on * the agent's own model); only a `reply` verdict proceeds to an agent turn. * - * When we do dispatch, we set `sourceReplyDeliveryMode: "message_tool_only"`, - * so the agent's final turn text is NOT auto-sent. The agent replies only by - * calling the message tool — and if it stays silent, nothing goes on the wire. - * That is what makes AgentChat a place where the agent *decides* what to do - * (reply, do something else, or nothing) instead of a chat interface that - * answers every turn. Two agents can no longer ping-pong by construction: a - * no-reply turn sends nothing, so the other side is never re-woken. + * The reply *gate* is the decision-maker and the loop-breaker: it runs before + * any turn, and a `no_reply` verdict sends nothing, so two agents can no longer + * ping-pong by construction. That is what makes AgentChat a place where the + * agent *decides* whether to engage instead of a chat interface that answers + * every turn. + * + * Once the gate says reply, the reply is delivered through our outbound. + * Delivery defaults to `automatic` (the agent's final turn text is sent) because + * it works regardless of the agent's tool profile. The stricter + * `message_tool_only` mode suppresses the turn text and makes the agent send via + * the `message` tool — but a restrictive profile (e.g. `coding`) strips that + * tool, which would leave such an agent unable to reply at all. The gate already + * prevents loops, so that mode is not needed for safety; operators whose agents + * keep the `message` tool can still opt in with + * `AGENTCHAT_SOURCE_REPLY_MODE=message_tool_only`. * * Non-text events (presence, typing, read receipts, rate-limit warnings, group * invites, group deletions) are surfaced through logs; they do NOT trigger a @@ -37,6 +45,7 @@ import type { AgentchatChannelRuntime } from '../runtime.js' import type { OpenClawConfig } from './openclaw-types.js' import { getThreadClosures } from './thread-closures.js' import { getClient } from './sdk-client.js' +import { hasAgentSendSince } from './send-tracker.js' import { decideReply, type GateCaller } from './gate.js' import type { GateInboundEvent, GateRawMessage, HistoryTurn } from './reply-gate.js' @@ -192,6 +201,14 @@ async function handleMessage( metadata: { reply_to: event.messageId }, }) } + // Single-send invariant (how Hermes avoids double-sends by construction: + // its invoker discards the turn text; the tool is the only wire path). Under + // `automatic` delivery the final turn text is our fallback reply — but when + // the agent already sent into this conversation during the turn (via + // `agentchat_send_message` or the core message tool), that final text is + // redundant self-narration ("I've responded to @peer…"). Deliver it ONLY + // when the turn produced no send of its own. + const turnStartMs = Date.now() const deliver = async (payload: { text?: string; blocks?: unknown[] }) => { if (threadClosures.isClosed(event.conversationId)) { deps.logger.info( @@ -200,6 +217,13 @@ async function handleMessage( ) return } + if (hasAgentSendSince(deps.accountId, event.conversationId, turnStartMs)) { + deps.logger.info( + { conversationId: event.conversationId, messageId: event.messageId }, + 'final turn text suppressed — agent already sent its reply via a message tool this turn', + ) + return + } await sendReply(payload.text ?? extractText(payload.blocks)) } @@ -248,7 +272,7 @@ async function handleMessage( if (!decision.reply) return } - // ── Dispatch (message_tool_only: agent sends via the tool, not auto) ── + // ── Dispatch the gated reply (mode per AGENTCHAT_SOURCE_REPLY_MODE) ── const { storePath, body: envelopeBody } = buildEnvelope({ channel: 'AgentChat', from: conversationLabel, @@ -292,10 +316,12 @@ async function handleMessage( recordInboundSession: session.recordInboundSession, dispatchReplyWithBufferedBlockDispatcher: channelRuntime.reply! .dispatchReplyWithBufferedBlockDispatcher! as never, - // The agent replies only by calling the message tool; its final turn - // text is not auto-delivered. This `deliver` fires only if the framework - // falls back to automatic source-reply delivery (e.g. message tool - // unavailable); a no_reply/silent turn sends nothing. + // Under `automatic` (the default) the framework hands the agent's final + // turn text to this `deliver`, which sends it to the source. Under the + // opt-in `message_tool_only` mode the turn text is suppressed and the + // agent sends via the message tool instead, so `deliver` only fires on a + // framework fallback. Either way a gated `no_reply` turn never runs, so + // nothing is sent. delivery: { deliver: async (payload) => { await deliver(payload as { text?: string; blocks?: unknown[] }) @@ -311,7 +337,7 @@ async function handleMessage( ) }, }, - replyOptions: { sourceReplyDeliveryMode: 'message_tool_only' }, + replyOptions: { sourceReplyDeliveryMode: resolveSourceReplyMode() }, record: { onRecordError: (err: unknown) => { deps.logger.error( @@ -374,6 +400,7 @@ async function runReplyGate(params: { ownHandle, nowMs, failOpen: gateFailOpen(), + timeoutMs: gateTimeoutMs(), caller: deps.gateCaller, }) } @@ -431,6 +458,7 @@ function readSeq(m: GateRawMessage): number { } const OFF_TOKENS = new Set(['0', 'false', 'off', 'no']) +const ON_TOKENS = new Set(['1', 'true', 'on', 'yes']) /** Reply gate kill switch — set `AGENTCHAT_REPLY_GATE_ENABLED=0` to disable. */ function gateEnabled(): boolean { @@ -438,12 +466,52 @@ function gateEnabled(): boolean { } /** - * On a gate failure, reply (fail-open, default) or stay silent (fail-closed via - * `AGENTCHAT_REPLY_GATE_FAIL_OPEN=0`). Fail-open is self-correcting: a dropped - * reply reads as a dead agent, and the gate re-runs on the next inbound. + * On a gate failure (model error / timeout / unparseable output), stay silent + * (fail-CLOSED, the default) or reply anyway (fail-open via + * `AGENTCHAT_REPLY_GATE_FAIL_OPEN=1`). + * + * Fail-closed is the safe default on a shared platform. If the model is down, a + * fail-open gate keeps voting "reply"; the compose then also fails and OpenClaw + * surfaces that error as a *sent* message, so two agents trade error messages + * forever — the very loop the gate exists to prevent. Silence under uncertainty + * cannot loop, and the gate re-runs on the next inbound once the model recovers. */ function gateFailOpen(): boolean { - return !OFF_TOKENS.has((process.env.AGENTCHAT_REPLY_GATE_FAIL_OPEN ?? '').trim().toLowerCase()) + return ON_TOKENS.has((process.env.AGENTCHAT_REPLY_GATE_FAIL_OPEN ?? '').trim().toLowerCase()) +} + +/** + * Decision-call timeout override (ms) via `AGENTCHAT_REPLY_GATE_TIMEOUT_MS`. + * The gate forces reasoning off so it's fast on any model, so the 20s default + * is generous; this is an escape hatch for genuinely slow self-hosted + * endpoints. A missing / non-numeric / non-positive value returns `undefined`, + * which lets the gate apply its own default (`DEFAULT_GATE_TIMEOUT_MS`). + */ +function gateTimeoutMs(): number | undefined { + const raw = process.env.AGENTCHAT_REPLY_GATE_TIMEOUT_MS + if (raw === undefined || raw.trim() === '') return undefined + const n = Number(raw) + return Number.isFinite(n) && n > 0 ? n : undefined +} + +/** Source-reply delivery modes OpenClaw supports for a channel turn. */ +type SourceReplyMode = 'automatic' | 'message_tool_only' + +/** + * Delivery mode for a gated reply. Default `automatic`: the gate already decided + * a reply is warranted, so the agent's final turn text is delivered through our + * outbound — which works no matter how the agent's tool profile is configured. + * + * `message_tool_only` (opt-in via `AGENTCHAT_SOURCE_REPLY_MODE=message_tool_only`) + * suppresses the turn text and requires the agent to send via the `message` + * tool. It gives the agent deliberate send control, but a restrictive tool + * profile (e.g. `coding`) strips that tool and the agent goes mute — so it is + * NOT the default. The reply gate, not this mode, is what prevents loops. + */ +function resolveSourceReplyMode(): SourceReplyMode { + return (process.env.AGENTCHAT_SOURCE_REPLY_MODE ?? '').trim().toLowerCase() === 'message_tool_only' + ? 'message_tool_only' + : 'automatic' } function handleGroupInvite(deps: InboundBridgeDeps, event: NormalizedGroupInvite): void { diff --git a/src/binding/outbound.ts b/src/binding/outbound.ts index abe8931..fff4072 100644 --- a/src/binding/outbound.ts +++ b/src/binding/outbound.ts @@ -46,6 +46,7 @@ import { parseChannelConfig } from '../config-schema.js' import { AgentChatChannelError } from '../errors.js' import { registerRuntime, getRuntime } from './runtime-registry.js' import { getClient } from './sdk-client.js' +import { recordAgentSend } from './send-tracker.js' import { createLogger } from '../log.js' function resolveConfig(cfg: OpenClawConfig | undefined, accountId?: string | null) { @@ -128,6 +129,10 @@ async function deliver( attachmentId, ) const result = await runtime.sendMessage(input) + // This adapter backs OpenClaw's core `message` tool (and CLI sends): an + // agent-initiated send. Record it so the inbound bridge can skip the + // redundant final-text delivery for the turn (single-send invariant). + recordAgentSend(accountId, result.message.conversation_id) return { channel: AGENTCHAT_CHANNEL_ID, messageId: result.message.id, diff --git a/src/binding/reply-gate.ts b/src/binding/reply-gate.ts index 64cb8de..7c9b5fe 100644 --- a/src/binding/reply-gate.ts +++ b/src/binding/reply-gate.ts @@ -214,6 +214,22 @@ function isRecord(value: unknown): value is GateRawMessage { // ─── Prompt construction ──────────────────────────────────────────────── +/** + * The done-ness criterion. This framing is the load-bearing part — it is the + * intervention the Hermes loop-sim validated (arm `two_gate`), and its rules + * exist because softer framings measurably fail: + * + * - Judge DONE-NESS, never value. "Would a reply be valuable?" always answers + * yes to a model — every riff feels valuable — which is precisely what feeds + * the exploration/"riffing" loop the ack-rules alone cannot stop. + * - Silence is success. Without this, the model's built-in continuation bias + * treats no_reply as a failure state and avoids it. + * - A reciprocal courtesy question is part of the pleasantry, not an open + * task. Two polite agents each end every turn with "and you?" — a literal + * "unanswered question directed at you" — and interview each other forever. + * This was the exact hole we watched live: every gate call scored + * open_request and the thread never terminated. + */ function systemTemplate(handle: string): string { const h = `@${handle}` return ( @@ -222,27 +238,43 @@ function systemTemplate(handle: string): string { `Your only job is to decide whether ${h} should reply to it now. You ` + `do NOT write the reply — you output one decision.\n` + `\n` + - `Choose "no_reply" when the exchange is finished or nothing actually needs ` + - `${h} to respond. For example:\n` + + `"no_reply" is a SUCCESS, not a failure. Most healthy conversations are ` + + `SUPPOSED to end; going quiet is the normal, correct outcome and is never ` + + `rude here — on this network silence IS the acknowledgement.\n` + + `\n` + + `Judge DONE-NESS, not how interesting another message could be:\n` + + `\n` + + `Choose "reply" only if a further message accomplishes a concrete, ` + + `still-OPEN purpose:\n` + + `- answer a substantive pending question or supply specifically requested ` + + `information\n` + + `- make or respond to a decision, or unblock the peer on a real task\n` + + `- ${h} started this thread toward a goal and the peer's reply needs a ` + + `substantive follow-up to reach it\n` + + `- new information genuinely requires ${h}'s input\n` + + `\n` + + `Choose "no_reply" when the exchange has reached its natural end — even if ` + + `another clever or friendly message is easily possible:\n` + + `- it is trading pleasantries, mutual appreciation, agreement, or ` + + `open-ended tangents / "riffing" with no open objective\n` + + `- a courtesy question that merely mirrors the exchange back ("and you?", ` + + `"what are you working on?", "what tools are you using?") after the ` + + `substantive part has run its course is part of the pleasantry, not an ` + + `open task — it does not oblige a reply\n` + `- the other side is acknowledging or closing out (thanks / ok / got it / ` + `sounds good / 👍 / bye) and replying would only prolong it\n` + - `- the last message is a pleasantry or reaction with no question, request, ` + - `or new information for ${h}\n` + `- ${h} already answered what was asked and nothing new is on the table\n` + - `- in a group, the message is not addressed to ${h} and does not need it\n` + - `\n` + - `Choose "reply" only when there is a real reason to respond. For example:\n` + - `- an open question, request, or task is directed at ${h} and unanswered\n` + - `- new information genuinely calls for ${h}'s input\n` + - `- ${h} started this and the peer's reply needs a substantive ` + - `follow-up to reach the goal\n` + + `- in a group, the message is not addressed to ${h} and does not need it. ` + + `(Groups only — in a direct conversation every message is addressed to ` + + `${h} by definition, so "not_addressed" never applies there.)\n` + `\n` + - `Decisive bias: once a conversation is winding down, prefer "no_reply" — a ` + - `reply must earn its place. Two agents trading acknowledgements forever is ` + - `the exact failure you exist to prevent. If the only thing you could add is ` + - `another acknowledgement, choose "no_reply". If the Pace line shows ` + - `messages flying back and forth rapidly with each only restating or ` + - `acknowledging the last, that IS the loop — choose "no_reply".\n` + + `"I could add something" is NOT a reason to reply. "Something concrete is ` + + `unresolved and my reply resolves it" IS. Two agents keeping a chat alive ` + + `by each politely asking the next question is the exact failure you exist ` + + `to prevent — the thread being pleasant does not make it open. If the Pace ` + + `line shows messages flying back and forth with each turn only restating, ` + + `appreciating, or re-asking a mirrored question, that IS the loop — choose ` + + `"no_reply". When unsure, prefer "no_reply".\n` + `\n` + `Respond with ONLY a JSON object — no prose, no markdown fences:\n` + `{"decision": "reply" or "no_reply", "reason": "", ` + diff --git a/src/binding/send-tracker.ts b/src/binding/send-tracker.ts new file mode 100644 index 0000000..1379917 Binary files /dev/null and b/src/binding/send-tracker.ts differ diff --git a/tests/binding/gate.test.ts b/tests/binding/gate.test.ts index 362264e..ef5f4eb 100644 --- a/tests/binding/gate.test.ts +++ b/tests/binding/gate.test.ts @@ -55,7 +55,8 @@ describe('decideReply', () => { ) expect(d.reply).toBe(true) expect(d.source).toBe('fail_open') - expect(d.reason).toBe('decision_call_error') + expect(d.reason).toContain('decision_call_error') + expect(d.reason).toContain('provider down') // underlying cause is surfaced }) it('fails closed (silent) when the decision call throws and failOpen is false', async () => { diff --git a/tests/binding/inbound-gate.test.ts b/tests/binding/inbound-gate.test.ts index b95e00c..7efaaf8 100644 --- a/tests/binding/inbound-gate.test.ts +++ b/tests/binding/inbound-gate.test.ts @@ -31,6 +31,10 @@ vi.mock('openclaw/plugin-sdk/inbound-reply-dispatch', () => ({ import { createInboundBridge } from '../../src/binding/inbound-bridge.js' import { resetThreadClosuresForTest } from '../../src/binding/thread-closures.js' +import { + recordAgentSend, + resetSendTrackerForTest, +} from '../../src/binding/send-tracker.js' import type { GateCaller } from '../../src/binding/gate.js' import type { NormalizedMessage } from '../../src/inbound.js' import type { AgentchatChannelConfig } from '../../src/config-schema.js' @@ -92,12 +96,12 @@ function makeChannelRuntime(): unknown { } } -function makeBridge(gateCaller: GateCaller) { +function makeBridge(gateCaller: GateCaller, runtime = makeRuntimeStub()) { return createInboundBridge({ accountId: 'default', config, logger, - runtime: makeRuntimeStub(), + runtime, channelRuntime: makeChannelRuntime() as never, gatewayCfg: {}, selfHandle: 'self-agent', @@ -112,32 +116,61 @@ describe('inbound reply gate', () => { beforeEach(() => { process.env.OPENCLAW_PROFILE = `inbound-gate-${Math.random().toString(36).slice(2)}` delete process.env.AGENTCHAT_REPLY_GATE_ENABLED // gate on by default + delete process.env.AGENTCHAT_REPLY_GATE_FAIL_OPEN // fail-closed by default + delete process.env.AGENTCHAT_SOURCE_REPLY_MODE // automatic by default recordSpy.mockClear() }) afterEach(() => { resetThreadClosuresForTest() + resetSendTrackerForTest() delete process.env.OPENCLAW_PROFILE delete process.env.AGENTCHAT_REPLY_GATE_ENABLED + delete process.env.AGENTCHAT_REPLY_GATE_FAIL_OPEN + delete process.env.AGENTCHAT_SOURCE_REPLY_MODE }) + function dispatchedMode(): string | undefined { + const arg = recordSpy.mock.calls[0]?.[0] as + | { replyOptions?: { sourceReplyDeliveryMode?: string } } + | undefined + return arg?.replyOptions?.sourceReplyDeliveryMode + } + it('does NOT dispatch a turn when the gate says no_reply', async () => { const bridge = makeBridge(noReplyCaller) await bridge(makeMessage()) expect(recordSpy).not.toHaveBeenCalled() }) - it('dispatches with message_tool_only when the gate says reply', async () => { + it('dispatches with automatic delivery by default when the gate says reply', async () => { + // automatic so the gated reply lands even when the agent's tool profile + // strips the message tool (the loop is already prevented by the gate). const bridge = makeBridge(replyCaller) await bridge(makeMessage()) expect(recordSpy).toHaveBeenCalledTimes(1) - const arg = recordSpy.mock.calls[0]?.[0] as - | { replyOptions?: { sourceReplyDeliveryMode?: string } } - | undefined - expect(arg?.replyOptions?.sourceReplyDeliveryMode).toBe('message_tool_only') + expect(dispatchedMode()).toBe('automatic') }) - it('fails open (dispatches) when the gate caller throws', async () => { + it('uses message_tool_only delivery when opted in via env', async () => { + process.env.AGENTCHAT_SOURCE_REPLY_MODE = 'message_tool_only' + const bridge = makeBridge(replyCaller) + await bridge(makeMessage()) + expect(recordSpy).toHaveBeenCalledTimes(1) + expect(dispatchedMode()).toBe('message_tool_only') + }) + + it('fails CLOSED (no dispatch) by default when the gate caller throws', async () => { + // A model outage must not reseed a loop: silence under uncertainty. + const bridge = makeBridge(async () => { + throw new Error('provider down') + }) + await bridge(makeMessage()) + expect(recordSpy).not.toHaveBeenCalled() + }) + + it('fails open (dispatches) when opted in via env and the caller throws', async () => { + process.env.AGENTCHAT_REPLY_GATE_FAIL_OPEN = '1' const bridge = makeBridge(async () => { throw new Error('provider down') }) @@ -159,4 +192,64 @@ describe('inbound reply gate', () => { await bridge(makeMessage({ conversationKind: 'group', conversationId: 'group_abc' })) expect(recordSpy).not.toHaveBeenCalled() }) + + // ── Single-send invariant ───────────────────────────────────────────── + // Hermes never double-sends: its invoker discards the turn text and the + // tool is the only wire path. Our equivalent: the final-turn-text delivery + // runs ONLY when the turn produced no send of its own. + + it('suppresses the final turn text when the agent already sent via a tool this turn', async () => { + recordSpy.mockImplementationOnce(async (params: unknown) => { + // Simulate the agent turn: a message-tool send lands mid-turn, then the + // framework hands the final turn text (self-narration) to `deliver`. + recordAgentSend('default', 'conv_abc', Date.now()) + const p = params as { delivery: { deliver: (x: { text: string }) => Promise } } + await p.delivery.deliver({ text: "I've responded to @peer-agent!" }) + }) + const runtime = makeRuntimeStub() + const bridge = makeBridge(replyCaller, runtime) + await bridge(makeMessage()) + expect(recordSpy).toHaveBeenCalledTimes(1) + expect(runtime.sendMessage).not.toHaveBeenCalled() + }) + + it('delivers the final turn text when the turn made no send of its own', async () => { + recordSpy.mockImplementationOnce(async (params: unknown) => { + const p = params as { delivery: { deliver: (x: { text: string }) => Promise } } + await p.delivery.deliver({ text: 'Paris' }) + }) + const runtime = makeRuntimeStub() + const bridge = makeBridge(replyCaller, runtime) + await bridge(makeMessage()) + expect(runtime.sendMessage).toHaveBeenCalledTimes(1) + const arg = (runtime.sendMessage as ReturnType).mock.calls[0]?.[0] as { + content?: { text?: string } + } + expect(arg?.content?.text).toBe('Paris') + }) + + it('does not suppress based on sends from before this turn', async () => { + // A send in a previous turn must not swallow this turn's reply. + recordAgentSend('default', 'conv_abc', Date.now() - 60_000) + recordSpy.mockImplementationOnce(async (params: unknown) => { + const p = params as { delivery: { deliver: (x: { text: string }) => Promise } } + await p.delivery.deliver({ text: 'a fresh reply' }) + }) + const runtime = makeRuntimeStub() + const bridge = makeBridge(replyCaller, runtime) + await bridge(makeMessage()) + expect(runtime.sendMessage).toHaveBeenCalledTimes(1) + }) + + it('does not suppress when the mid-turn send went to a different conversation', async () => { + recordSpy.mockImplementationOnce(async (params: unknown) => { + recordAgentSend('default', 'conv_other', Date.now()) + const p = params as { delivery: { deliver: (x: { text: string }) => Promise } } + await p.delivery.deliver({ text: 'reply to the origin thread' }) + }) + const runtime = makeRuntimeStub() + const bridge = makeBridge(replyCaller, runtime) + await bridge(makeMessage()) + expect(runtime.sendMessage).toHaveBeenCalledTimes(1) + }) }) diff --git a/tests/binding/reply-gate.test.ts b/tests/binding/reply-gate.test.ts index a3e97c6..b28ad9b 100644 --- a/tests/binding/reply-gate.test.ts +++ b/tests/binding/reply-gate.test.ts @@ -123,6 +123,19 @@ describe('buildDecisionMessages', () => { expect(out[0]?.content).toContain('reply gate for @me') }) + it('carries the done-ness criteria the loop-sim validated (anti-riffing)', () => { + // These lines are load-bearing: the loop-sim proved value-framed criteria + // never stop the riffing loop. Removing any of them regresses the fix. + const sys = buildDecisionMessages({ handle: 'me', event, history: [], signals })[0] + ?.content as string + expect(sys).toContain('"no_reply" is a SUCCESS') + expect(sys).toContain('Judge DONE-NESS') + expect(sys).toContain('riffing') + expect(sys).toContain('courtesy question') + expect(sys).toContain('"I could add something" is NOT a reason to reply') + expect(sys).toContain('When unsure, prefer "no_reply"') + }) + it('includes a pace line when a previous-message gap exists', () => { const out = buildDecisionMessages({ handle: 'me', event, history: [], signals }) expect(out[1]?.content).toContain('Pace: 4 message(s) in the last 60s') diff --git a/tests/binding/send-tracker.test.ts b/tests/binding/send-tracker.test.ts new file mode 100644 index 0000000..eb421d1 --- /dev/null +++ b/tests/binding/send-tracker.test.ts @@ -0,0 +1,46 @@ +import { afterEach, describe, it, expect } from 'vitest' + +import { + recordAgentSend, + hasAgentSendSince, + resetSendTrackerForTest, +} from '../../src/binding/send-tracker.js' + +describe('send-tracker', () => { + afterEach(() => { + resetSendTrackerForTest() + }) + + it('reports a send at or after the given start time', () => { + recordAgentSend('default', 'conv_a', 1_000) + expect(hasAgentSendSince('default', 'conv_a', 1_000)).toBe(true) + expect(hasAgentSendSince('default', 'conv_a', 999)).toBe(true) + expect(hasAgentSendSince('default', 'conv_a', 1_001)).toBe(false) + }) + + it('is scoped by conversation and account', () => { + recordAgentSend('default', 'conv_a', 1_000) + expect(hasAgentSendSince('default', 'conv_b', 0)).toBe(false) + expect(hasAgentSendSince('other', 'conv_a', 0)).toBe(false) + }) + + it('ignores empty conversation ids', () => { + recordAgentSend('default', '', 1_000) + expect(hasAgentSendSince('default', '', 0)).toBe(false) + }) + + it('keeps the newest timestamp for a conversation', () => { + recordAgentSend('default', 'conv_a', 1_000) + recordAgentSend('default', 'conv_a', 2_000) + expect(hasAgentSendSince('default', 'conv_a', 1_500)).toBe(true) + }) + + it('prunes the stalest entries beyond the cap without losing fresh ones', () => { + for (let i = 0; i < 600; i++) { + recordAgentSend('default', `conv_${i}`, i) + } + // Freshest survives; the very first entries were pruned. + expect(hasAgentSendSince('default', 'conv_599', 0)).toBe(true) + expect(hasAgentSendSince('default', 'conv_0', 0)).toBe(false) + }) +})