diff --git a/changelog.d/skip-reply-wake-values.fixed.md b/changelog.d/skip-reply-wake-values.fixed.md new file mode 100644 index 000000000..d798a0c51 --- /dev/null +++ b/changelog.d/skip-reply-wake-values.fixed.md @@ -0,0 +1,4 @@ +- `skip_reply` reads `wake_in_seconds` one way for every part of the host: a number of seconds from 1 to 3600, or a string that reads as one, taken in whole seconds. A larger number waits 3600, `Infinity` included, where before `Infinity` armed nothing and the turn ended as an ordinary skip. The tool's description of the parameter now says "from 1 to 3600; a larger number waits 3600" in place of "clamped to 1–3600". +- With an EventGate, a `wake_in_seconds` that isn't such a number is refused as a tool error that names the value. That covers a number below 1, such as `0`, a negative or `0.5`, as well as `"soon"`, `""`, `true`, an array or an object. It arms nothing and doesn't end the turn. Before, `0`, negatives and `"soon"` ended the turn with no wake, a number between 0 and 1 such as `0.5` armed a wake of 1 s, and `true` and `[5]` armed wakes of 1 s and 5 s. A number below 1 is refused rather than raised to 1, so that `0`, which a model may write to fill an optional number, can't become a 1-second wake after every turn. +- Without an EventGate, any `wake_in_seconds` other than absent or `null` is now refused, as a positive number, `true` and `[5]` already were. `0`, negatives, `Infinity`, a blank string and values `Number()` can't read used to end the turn as an ordinary skip, which dropped the wake the call asked for without saying so. +- A `skip_reply` call whose input is `null` ends the turn as an ordinary skip, where the channel registry's handler threw reading `wake_in_seconds`. diff --git a/src/framework.ts b/src/framework.ts index 394ccd81e..f5a5873f7 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -86,6 +86,7 @@ import { ToolLifecycleEmitter, parseToolObserveParams, type ScriptCallOrigin } f import { checkToolPattern, describeUnmatchedPattern, type PatternServer } from './mcpl/tool-pattern-check.js'; import { BUILTIN_TOOL_CLASSES, normalizeClassTable, parseDeclaredClasses, resolveToolClass, type EffectiveToolClass, type ToolClass, type ToolClassSource } from './mcpl/tool-classes.js'; import { PushHandler, type McplPushEvent } from './mcpl/push-handler.js'; +import { readSkipReplyWake, skipReplyWakeRefusal } from './mcpl/skip-reply-wake.js'; import { convertBlock } from './mcpl/convert-block.js'; import { PushCoalescer, CoalesceError, PUSH_COALESCING_SUPPORT, validateCoalesceMember, validateCoalescedContent, @@ -178,8 +179,8 @@ function sniffImageMediaType(data: Buffer): string | undefined { * `send_message` followed by "sent it" from double-posting, but a new message * starts a new conversational round and must be answerable with plain prose. * `skip_reply` ends the turn at the tool-result boundary, unless it's refused - * (wake_in_seconds without an EventGate): then the turn goes on, and only its - * own round stays silent. + * (a wake_in_seconds without an EventGate, or one that isn't a number of + * seconds): then the turn goes on, and only its own round stays silent. */ const SILENCING_TOOLS = new Set([ 'skip_reply', 'channel_publish', 'send_message', 'reply_message', 'send_dm', @@ -236,18 +237,6 @@ const silenceCauseOf = (calls: Array<{ name: string; input?: unknown }>): 'send' return calls.some(isSilencingCall) ? 'send' : null; }; -/** - * The seconds a skip_reply call asks to be woken after: its wake_in_seconds - * read as a number, when that's finite and above 0, and undefined when it asks - * for no timed wake. Without an EventGate such a call is refused, so the - * dispatch that refuses it and the round that keeps its prose private both - * read the request here. - */ -const requestedSelfWakeSeconds = (input: unknown): number | undefined => { - const secs = Number((input as Record | null | undefined)?.wake_in_seconds); - return Number.isFinite(secs) && secs > 0 ? secs : undefined; -}; - /** * True when an injected message is real conversational input — something the * agent might actually be replying to — rather than ambient machinery. @@ -18545,15 +18534,23 @@ export class AgentFramework { } /** - * Whether this host refuses a skip_reply call: it asks for a timed wake - * (requestedSelfWakeSeconds), the channel registry handles skip_reply, and - * there's no EventGate to set the wake. The dispatch that refuses the call, - * the round that keeps its prose private and the turn's silencing all read - * it here, for the model's own calls. + * Why this host refuses a skip_reply call, as the model will read it, or + * undefined when it doesn't (skipReplyWakeRefusal): without an EventGate, + * any wake_in_seconds; with one, a wake_in_seconds that isn't a number of + * seconds. Only when the channel registry handles skip_reply. The dispatch + * that refuses the call reads it here, and so does the turn's bookkeeping of + * a refused round, for the model's own calls: the silence before it, which + * comes back after its round, and in XML mode which round's prose stays + * private. A skip_reply round's own prose stays private by the tool's name, + * refused or not. */ + private skipReplyRefusal(call: { name?: string; input?: unknown }): string | undefined { + if (call.name !== 'skip_reply' || !this.channelRegistry) return undefined; + return skipReplyWakeRefusal(readSkipReplyWake(call.input), !!this.eventGate); + } + private refusesSelfWake(call: { name?: string; input?: unknown }): boolean { - return call.name === 'skip_reply' && !!this.channelRegistry && !this.eventGate - && requestedSelfWakeSeconds(call.input) !== undefined; + return this.skipReplyRefusal(call) !== undefined; } /** @@ -18680,21 +18677,25 @@ export class AgentFramework { // the agent after N seconds unless something else wakes it first // (any turn start cancels the pending self-wake). Armed here because // the framework owns the EventGate; the registry only words the - // confirmation. Without a gate, a positive timer request is refused so - // the model learns that its turn remains open and no wake was armed. + // confirmation. A wake the host can't set is refused, so the model + // learns that its turn remains open and no wake was armed: any + // wake_in_seconds without a gate, and one that isn't a number of + // seconds with one. if (enrichedCall.name === 'skip_reply') { - const secs = requestedSelfWakeSeconds(enrichedCall.input); - if (secs !== undefined && this.eventGate) { - const { inMs } = this.eventGate.armSelfWake(agentName, secs, 'skip_reply'); - console.error(`[self-wake] agent=${agentName} armed in ${Math.round(inMs / 1000)}s (skip_reply)`); - } else if (this.refusesSelfWake(enrichedCall)) { - const error = "Your turn didn't end, and no wake was set: this host can't wake you on a timer, so skip_reply can't take wake_in_seconds here. To end the turn and wait for the next message, call skip_reply without it. Anything you meant to come back to, do now."; - console.error(`[self-wake] agent=${agentName} No EventGate configured (FrameworkConfig.gate is unset). ${error}`); + const error = this.skipReplyRefusal(enrichedCall); + if (error !== undefined) { + const wakeValue = (enrichedCall.input as { wake_in_seconds?: unknown } | null | undefined)?.wake_in_seconds; + console.error(`[self-wake] agent=${agentName} wake_in_seconds=${logValue(wakeValue)} skip_reply refused: ${logDetail(this.eventGate ? "wake_in_seconds isn't a number of seconds" : 'No EventGate configured (FrameworkConfig.gate is unset)')}`); this.toolLifecycleEmitter?.refuse(agentName, enrichedCall.id); this.emitTrace({ type: 'tool:failed', module: 'channels', tool: enrichedCall.name, callId: enrichedCall.id, error }); this.pushEvent({ type: 'tool-result', callId: enrichedCall.id, toolName: enrichedCall.name, agentName, moduleName: 'channels', result: { success: false, error, isError: true } }); return; } + const wake = readSkipReplyWake(enrichedCall.input); + if (wake.kind === 'wake' && this.eventGate) { + const { inMs } = this.eventGate.armSelfWake(agentName, wake.seconds, 'skip_reply'); + console.error(`[self-wake] agent=${agentName} armed in ${Math.round(inMs / 1000)}s (skip_reply)`); + } } this.dispatchChannelToolCall(agentName, enrichedCall); return; diff --git a/src/mcpl/channel-registry.ts b/src/mcpl/channel-registry.ts index 8c906bfa6..338da5f87 100644 --- a/src/mcpl/channel-registry.ts +++ b/src/mcpl/channel-registry.ts @@ -44,6 +44,7 @@ import { RefusalLines } from './refusal-lines.js'; import { CapabilityGrant } from './capability-grant.js'; import type { InboundSource } from './inbound-source.js'; import { McplRequestError } from './server-connection.js'; +import { readSkipReplyWake } from './skip-reply-wake.js'; // ============================================================================ // Typing indicator interval (Discord typing lasts ~10s, so 7s keeps it alive) @@ -382,8 +383,9 @@ const CHANNEL_TOOL_DEFINITIONS: ToolDefinition[] = [ type: 'number', description: 'Optional self-wake: if nothing else wakes you first, you wake again after this ' + - 'many seconds (1 = go again almost immediately; clamped to 1–3600). Any other ' + - 'wake before then cancels it. Omit to stay idle until the next external wake.', + 'many seconds, from 1 to 3600; a larger number waits 3600 (1 = go again almost ' + + 'immediately). Any other wake before then cancels it. Omit to stay idle until the ' + + 'next external wake.', }, }, required: [], @@ -1515,7 +1517,7 @@ export class ChannelRegistry { return this.handleToolJournal(input as { content?: string }); case 'skip_reply': - return this.handleToolSkipReply(input as { reason?: string; wake_in_seconds?: number }); + return this.handleToolSkipReply(input as { reason?: string; wake_in_seconds?: unknown }); default: return { success: false, error: `Unknown channel tool: ${toolName}`, isError: true }; @@ -3032,14 +3034,15 @@ export class ChannelRegistry { * tool, so any trailing prose this turn is NOT posted. Replaces the old * overloaded use of `think` for staying silent. */ - private handleToolSkipReply(input: { reason?: string; wake_in_seconds?: number }): ToolResult { + private handleToolSkipReply(input: { reason?: string; wake_in_seconds?: unknown }): ToolResult { // The self-wake itself is armed by the framework (which owns the - // EventGate) BEFORE this dispatch. Without a gate, a positive timer request - // is refused before this successful end-turn handler is reached. - const wakeSecs = Number(input.wake_in_seconds); - const selfWake = Number.isFinite(wakeSecs) && wakeSecs > 0 - ? Math.max(1, Math.min(3600, Math.floor(wakeSecs))) - : undefined; + // EventGate) BEFORE its dispatch reaches this handler, from the same + // reading, and the framework refuses a wake the host can't set before + // dispatching. A caller of this registry's handleChannelToolCall + // (AgentFramework.channels) goes past both, so its note names a wake + // nothing armed. + const wake = readSkipReplyWake(input); + const selfWake = wake.kind === 'wake' ? wake.seconds : undefined; return { success: true, // The note says "ended the turn" — make it TRUE. Without endTurn the diff --git a/src/mcpl/skip-reply-wake.ts b/src/mcpl/skip-reply-wake.ts new file mode 100644 index 000000000..4932823be --- /dev/null +++ b/src/mcpl/skip-reply-wake.ts @@ -0,0 +1,73 @@ +/** + * How `skip_reply` reads its `wake_in_seconds`, for every site that acts on + * it: the framework arms the EventGate's timer or refuses the call, the turn's + * bookkeeping of a refused round asks whether it was refused, and the channel + * registry words the confirmation. One reading, so a value can't arm a timer + * at one site and read as no wake at another, and the confirmation names the + * seconds the timer is set for. + */ +import { safeSlice } from '../safe-slice.js'; + +/** The range the tool's schema offers, in seconds: a larger number waits the most. */ +export const SKIP_REPLY_WAKE_MIN_SECONDS = 1; +export const SKIP_REPLY_WAKE_MAX_SECONDS = 3600; + +/** + * What a `skip_reply` call's `wake_in_seconds` asks for: + * - `none`: absent or `null`, an ordinary skip; + * - `wake`: a number of at least 1, or a string that `Number()` reads as one, + * in whole seconds and at most 3600, so 90.7 waits 90 s and anything over + * 3600, Infinity included, waits 3600 s; + * - `invalid`: anything else, which asks for no number of seconds the host can + * set: a number below 1, such as 0, a negative or 0.5, and "soon", "", + * `true`, an array or an object. Below 1 is refused rather than raised to 1: + * 0 is a value a model may write when it fills an optional number with a + * default, and as a 1 s wake it would wake the agent again a second after + * each turn, for as long as it kept skipping with 0. + */ +export type SkipReplyWake = + | { kind: 'none' } + | { kind: 'wake'; seconds: number } + | { kind: 'invalid'; value: unknown }; + +export function readSkipReplyWake(input: unknown): SkipReplyWake { + const value = input !== null && typeof input === 'object' + ? (input as Record).wake_in_seconds + : undefined; + if (value === undefined || value === null) return { kind: 'none' }; + // A blank string reads as 0, so it's refused as a number below 1. + const seconds = typeof value === 'number' ? value + : typeof value === 'string' ? Number(value) + : Number.NaN; + if (Number.isNaN(seconds) || seconds < SKIP_REPLY_WAKE_MIN_SECONDS) return { kind: 'invalid', value }; + return { kind: 'wake', seconds: Math.floor(Math.min(SKIP_REPLY_WAKE_MAX_SECONDS, seconds)) }; +} + +/** The refusal the model reads when its host has no EventGate to set a timer. */ +export const SKIP_REPLY_WAKE_NO_GATE_REFUSAL = + "Your turn didn't end, and no wake was set: this host can't wake you on a timer, so skip_reply can't take wake_in_seconds here. To end the turn and wait for the next message, call skip_reply without it. Anything you meant to come back to, do now."; + +/** + * Why a `skip_reply` call is refused, as the model will read it, or undefined + * when it isn't: without an EventGate, any `wake_in_seconds` but absent or + * `null`, since the host offers none; with one, a value that isn't a number of + * seconds. A refused call arms nothing and doesn't end the turn. + */ +export function skipReplyWakeRefusal(wake: SkipReplyWake, gatePresent: boolean): string | undefined { + if (wake.kind === 'none') return undefined; + if (!gatePresent) return SKIP_REPLY_WAKE_NO_GATE_REFUSAL; + if (wake.kind === 'wake') return undefined; + return `Your turn didn't end, and no wake was set: wake_in_seconds takes a number of seconds from ${SKIP_REPLY_WAKE_MIN_SECONDS} to ${SKIP_REPLY_WAKE_MAX_SECONDS}, and ${describeValue(wake.value)} isn't one. Call skip_reply with a number of seconds to come back on your own, or without wake_in_seconds to end the turn and wait for the next message.`; +} + +/** + * A value as the model wrote it, as JSON, cut to keep the refusal short, + * never between the halves of a surrogate pair. A tool call's input is parsed + * JSON, so it always has a JSON text, but JSON writes a number that isn't + * finite as `null`, and `-1e400` parses as -Infinity, so a number prints as + * `String()` of it. + */ +function describeValue(value: unknown): string { + const text = typeof value === 'number' ? String(value) : JSON.stringify(value); + return text.length > 80 ? `${safeSlice(text, 0, 80)}…` : text; +} diff --git a/test/skip-reply-wake-capability.test.ts b/test/skip-reply-wake-capability.test.ts index 2e4449f4f..8f3048642 100644 --- a/test/skip-reply-wake-capability.test.ts +++ b/test/skip-reply-wake-capability.test.ts @@ -255,19 +255,32 @@ describe('skip_reply wake capability', () => { }); -describe('refused wake: no lifecycle events; zero is an ordinary skip', () => { - async function runProbe(wake: unknown) { +describe('refused wake: no lifecycle events; a wake_in_seconds the host can\'t set is refused, never dropped', () => { + // One turn whose first round is a skip_reply carrying `wake` (or, with + // `call`, another tool or another input), then a round of plain text. + async function runProbe(wake: unknown, gate = false, call?: { name: string; input: unknown }) { const dir = mkdtempSync(join(tmpdir(), 'skip-wake-probe-')); const membrane = new MockMembrane(); + const { name, input } = call ?? { name: 'skip_reply', input: { reason: 'later', wake_in_seconds: wake } }; membrane.pushResponse(createMockResponse([ { type: 'text', text: 'same-round text must stay private' }, - { type: 'tool_use', id: 'wake-1', name: 'skip_reply', input: { reason: 'later', wake_in_seconds: wake } }, + { type: 'tool_use', id: 'wake-1', name, input }, ] as ContentBlock[], 'tool_use')); membrane.pushResponse(createMockResponse([{ type: 'text', text: 'next-round reply' }] as ContentBlock[])); const framework = await AgentFramework.create({ storePath: join(dir, 'store'), membrane: membrane.asMembrane(), agents: [{ name: 'agent', model: 'test', systemPrompt: 'test' }], modules: [], + ...(gate ? { gate: { configPath: join(dir, 'gate.json') } } : {}), }); + // The seconds the framework asks the gate to wake after, as it asks. + const armed: number[] = []; + const eventGate = (framework as unknown as { + eventGate?: { armSelfWake(agent: string, seconds: number, source?: string): { inMs: number } }; + }).eventGate; + if (eventGate) { + const arm = eventGate.armSelfWake.bind(eventGate); + eventGate.armSelfWake = (agent, seconds, source) => { armed.push(seconds); return arm(agent, seconds, source); }; + } const lifecycle: ToolLifecycleParams[] = []; const observer = { id: 'observer', @@ -278,10 +291,18 @@ describe('refused wake: no lifecycle events; zero is an ordinary skip', () => { observers: () => [observer], configFor: () => undefined, describe: () => ({ class: [] }), }); const routed: string[] = []; + const handled: string[] = []; (framework as unknown as { channelRegistry: unknown }).channelRegistry = new Proxy({ getChannelTools: () => [], resolveLocus: () => 'channel:test', routeSpeech: async (_agent: string, text: string) => { routed.push(text); return { delivered: true, channelId: 'channel:test' }; }, - handleChannelToolCall: async () => ({ success: true, endTurn: true, data: { skipped: true, note: 'Turn ended; nothing sent.' } }), + // What reached the registry: a skip_reply ends the turn, and a send + // silences the rest of it. + handleChannelToolCall: async (toolName: string) => { + handled.push(toolName); + return toolName === 'skip_reply' + ? { success: true, endTurn: true, data: { skipped: true, note: 'Turn ended; nothing sent.' } } + : { success: true, data: { delivered: true } }; + }, }, { get: (target, prop: string) => prop in target ? target[prop as keyof typeof target] : () => undefined }); const agent = framework.getAgent('agent')!; await (framework as unknown as { startAgentStream(agent: unknown, trigger: unknown): Promise }) @@ -290,7 +311,8 @@ describe('refused wake: no lifecycle events; zero is an ordinary skip', () => { const results = membrane.lastStream?.receivedToolResults ?? []; await framework.stop(); rmSync(dir, { recursive: true, force: true }); - return { lifecycle, routed, results }; + const refusal = results[0]?.[0] as { isError?: boolean; content?: string } | undefined; + return { lifecycle, routed, results, armed, refusal, handled }; } it('A: a refused positive wake sends no lifecycle events', async () => { @@ -300,20 +322,103 @@ describe('refused wake: no lifecycle events; zero is an ordinary skip', () => { assert.deepEqual(lifecycle.map((p) => `${p.toolCallId}:${p.phase}`), []); }); - it('B: wake_in_seconds: 0 remains an ordinary skip', async () => { - const { routed, results } = await runProbe(0); - assert.deepEqual(routed, []); - assert.equal(results.length, 0, 'ordinary skip ends at the tool boundary'); + it('B: without a gate, wake_in_seconds: 0 is refused like any other value', async () => { + const { routed, refusal } = await runProbe(0); + assert.equal(refusal?.isError, true); + assert.match(refusal?.content ?? '', /this host can't wake you on a timer/); + assert.deepEqual(routed, ['next-round reply'], 'the turn went on to its next round'); }); - it('C: a wake that doesn\'t read as a finite number asks for none, so it\'s an ordinary skip', async () => { + it('C: without a gate, a wake_in_seconds that isn\'t a number is refused too, not taken as an ordinary skip', async () => { // JSON reads a number literal past a double's range, such as 1e400, as // Infinity too, but the store can't hold an Infinity in a tool call's // input, so these are strings. - for (const wake of ['Infinity', 'soon']) { - const { routed, results } = await runProbe(wake); - assert.deepEqual(routed, [], `wake_in_seconds: ${String(wake)}`); - assert.equal(results.length, 0, `wake_in_seconds: ${String(wake)} ends the turn at the tool boundary`); + for (const wake of ['Infinity', 'soon', '']) { + const { routed, refusal } = await runProbe(wake); + assert.equal(refusal?.isError, true, `wake_in_seconds: ${JSON.stringify(wake)}`); + assert.match(refusal?.content ?? '', /this host can't wake you on a timer/, `wake_in_seconds: ${JSON.stringify(wake)}`); + assert.deepEqual(routed, ['next-round reply'], `wake_in_seconds: ${JSON.stringify(wake)}`); + } + }); + + it('D: null still asks for no wake, as does a call whose whole input is null: ordinary skips', async () => { + for (const [label, gate, call] of [ + ['wake_in_seconds: null, no gate', false, undefined], + ['wake_in_seconds: null, with a gate', true, undefined], + ['input null, with a gate', true, { name: 'skip_reply', input: null }], + ] as const) { + const { routed, results, handled, armed } = await runProbe(null, gate, call); + assert.deepEqual(handled, ['skip_reply'], `${label}: the call reached the registry's skip_reply`); + assert.deepEqual(routed, [], label); + assert.equal(results.length, 0, `${label}: an ordinary skip ends at the tool boundary`); + assert.deepEqual(armed, [], `${label}: nothing armed`); } }); + + it('E: with a gate, a wake_in_seconds that isn\'t a number of seconds is refused, named, and arms nothing', async () => { + for (const wake of [0, -5, 0.5, 'soon', '10 minutes', '', true, [5], {}]) { + const { routed, refusal, armed, lifecycle } = await runProbe(wake, true); + const label = `wake_in_seconds: ${JSON.stringify(wake)}`; + assert.equal(refusal?.isError, true, label); + assert.equal( + refusal?.content, + `Your turn didn't end, and no wake was set: wake_in_seconds takes a number of seconds from 1 to 3600, and ${JSON.stringify(wake)} isn't one. Call skip_reply with a number of seconds to come back on your own, or without wake_in_seconds to end the turn and wait for the next message.`, + label, + ); + assert.deepEqual(armed, [], `${label} arms nothing`); + assert.deepEqual(routed, ['next-round reply'], `${label}: the refused round stays private, and the turn goes on`); + assert.deepEqual(lifecycle.map((p) => `${p.toolCallId}:${p.phase}`), [], `${label} sends no lifecycle events`); + } + }); + + it('G: with a gate, a call to another tool that carries wake_in_seconds isn\'t a refused skip_reply', async () => { + // A send silences the rest of the turn; read as a refused skip_reply, it + // wouldn't, and the next round's reply would go out. + const { routed, results, handled, armed } = await runProbe(undefined, true, { + name: 'channel_publish', input: { channelId: 'channel:test', content: 'sent', wake_in_seconds: 'soon' }, + }); + assert.deepEqual(handled, ['channel_publish']); + assert.notEqual((results[0]?.[0] as { isError?: boolean } | undefined)?.isError, true, 'the send isn\'t refused'); + assert.deepEqual(routed, [], 'the send silences the rest of the turn'); + assert.deepEqual(armed, []); + }); + + it('F: with a gate, a number from 1, or a string that reads as one, arms a wake in whole seconds, and over 3600 waits 3600', async () => { + for (const [wake, seconds] of [[1, 1], [90.7, 90], ['30', 30], [5000, 3600], ['Infinity', 3600]] as const) { + const { routed, results, armed } = await runProbe(wake, true); + const label = `wake_in_seconds: ${JSON.stringify(wake)}`; + assert.deepEqual(armed, [seconds], label); + assert.equal(results.length, 0, `${label} ends the turn at the tool boundary`); + assert.deepEqual(routed, [], label); + } + }); +}); + +describe('the channel registry\'s skip_reply confirmation names the wake the gate sets', () => { + const note = async (wake: unknown) => { + const registry = new ChannelRegistry( + {} as McplServerRegistry, {} as FeatureSetManager, () => {}, () => {}, { eventGatePresent: true }, + ); + const input: Record = { reason: 'later' }; + if (wake !== undefined) input.wake_in_seconds = wake; + const result = await registry.handleChannelToolCall('skip_reply', input, { kind: 'agent', agentName: 'agent' }); + return (result as { data?: { note?: string } }).data?.note; + }; + + it('in whole seconds up to 3600, as the gate is set, and none when absent', async () => { + // Whole seconds, the seconds the framework arms the gate's timer for. + for (const [wake, seconds] of [[60, 60], [90.7, 90], ['30', 30], [1, 1], [5000, 3600], ['Infinity', 3600]] as const) { + assert.equal(await note(wake), `Turn ended; nothing sent. Self-wake in ~${seconds}s unless something wakes you first.`, `wake_in_seconds: ${JSON.stringify(wake)}`); + } + assert.equal(await note(undefined), 'Turn ended; nothing sent.'); + assert.equal(await note(null), 'Turn ended; nothing sent.'); + }); + + it('a call whose input is null is an ordinary skip, where it used to throw reading wake_in_seconds', async () => { + const registry = new ChannelRegistry( + {} as McplServerRegistry, {} as FeatureSetManager, () => {}, () => {}, { eventGatePresent: true }, + ); + const result = await registry.handleChannelToolCall('skip_reply', null, { kind: 'agent', agentName: 'agent' }); + assert.deepEqual(result, { success: true, endTurn: true, data: { skipped: true, note: 'Turn ended; nothing sent.' } }); + }); }); diff --git a/test/skip-reply-wake-xml-mode.test.ts b/test/skip-reply-wake-xml-mode.test.ts index 518f3659f..23edcf9a6 100644 --- a/test/skip-reply-wake-xml-mode.test.ts +++ b/test/skip-reply-wake-xml-mode.test.ts @@ -40,7 +40,7 @@ const echo = (id: string, text: string, ...more: string[]): Round => ({ calls: [id, ...more].map((callId) => ({ id: callId, name: 'test--echo', input: { message: 'x' } })), results: `${[id, ...more].map(() => '{"echoed":"x"}').join('')}`, }); -const refusedWake = (id: string, text: string): Round => ({ text, calls: [{ id, name: 'skip_reply', input: { reason: 'later', wake_in_seconds: 60 } }], results: 'refused' }); +const refusedWake = (id: string, text: string, wake: unknown = 60): Round => ({ text, calls: [{ id, name: 'skip_reply', input: { reason: 'later', wake_in_seconds: wake } }], results: 'refused' }); class XmlStream implements YieldingStream { private events: StreamEvent[] = []; @@ -118,8 +118,8 @@ class EchoModule implements Module { async onProcess(_event: ProcessEvent, _state: ProcessState): Promise { return {}; } } -/** One XML-mode turn with no EventGate: what reached the channel, and whether each round's result was an error. */ -async function xmlTurn(rounds: Round[], finalText: string, thinking = false) { +/** One XML-mode turn, with no EventGate unless asked: what reached the channel, and whether each round's result was an error. */ +async function xmlTurn(rounds: Round[], finalText: string, thinking = false, gate = false) { let stream: XmlStream | undefined; const membrane = { async complete(_r: NormalizedRequest): Promise { throw new Error('not used'); }, @@ -132,6 +132,7 @@ async function xmlTurn(rounds: Round[], finalText: string, thinking = false) { const framework = await AgentFramework.create({ storePath: join(dir, 'store'), membrane: membrane as unknown as import('@animalabs/membrane').Membrane, agents: [{ name: 'agent', model: 'test', systemPrompt: 'test' }], modules: [new EchoModule()], + ...(gate ? { gate: { configPath: join(dir, 'gate.json') } } : {}), }); const routed: string[] = []; (framework as unknown as { channelRegistry: unknown }).channelRegistry = new Proxy({ @@ -208,6 +209,12 @@ describe('skip_reply refused in XML tool mode: its round stays private, and the assert.deepEqual(r.routed, ['ONE', 'FINAL_REPLY']); }); + it('with a gate, a wake_in_seconds that isn\'t a number of seconds is refused the same way', async () => { + const r = await xmlTurn([echo('e1', 'ONE'), refusedWake('w1', 'REFUSED_PRIVATE', 'soon')], '\nFINAL_REPLY', false, true); + assert.deepEqual(r.errors, [false, true], 'the wake was refused and the turn went on'); + assert.deepEqual(r.routed, ['ONE', 'FINAL_REPLY']); + }); + it('a refused round with no prose keeps the next round\'s prose', async () => { const r = await xmlTurn([echo('e1', 'ONE'), refusedWake('w1', ''), echo('e2', 'TWO')], '\nTHIRD'); assert.deepEqual(r.errors, [false, true, false]); diff --git a/test/skip-reply-wake.test.ts b/test/skip-reply-wake.test.ts new file mode 100644 index 000000000..9bf3a9ba8 --- /dev/null +++ b/test/skip-reply-wake.test.ts @@ -0,0 +1,84 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { + SKIP_REPLY_WAKE_NO_GATE_REFUSAL, + readSkipReplyWake, + skipReplyWakeRefusal, +} from '../src/mcpl/skip-reply-wake.js'; + +const read = (wake: unknown) => readSkipReplyWake({ reason: 'later', wake_in_seconds: wake }); + +describe('readSkipReplyWake: what a skip_reply call\'s wake_in_seconds asks for', () => { + it('absent or null asks for no wake', () => { + assert.deepEqual(readSkipReplyWake({ reason: 'later' }), { kind: 'none' }); + assert.deepEqual(read(null), { kind: 'none' }); + assert.deepEqual(readSkipReplyWake(null), { kind: 'none' }, 'a call whose input is null'); + assert.deepEqual(readSkipReplyWake('wake'), { kind: 'none' }, 'a call whose input isn\'t an object'); + }); + + it('a number from 1 is the wake, in whole seconds', () => { + assert.deepEqual(read(60), { kind: 'wake', seconds: 60 }); + assert.deepEqual(read(1), { kind: 'wake', seconds: 1 }); + assert.deepEqual(read(1.5), { kind: 'wake', seconds: 1 }); + assert.deepEqual(read(90.7), { kind: 'wake', seconds: 90 }); + }); + + it('a string that reads as a number is that number', () => { + assert.deepEqual(read('30'), { kind: 'wake', seconds: 30 }); + assert.deepEqual(read(' 30 '), { kind: 'wake', seconds: 30 }); + }); + + it('a number over 3600 waits 3600, as the schema says, Infinity included', () => { + for (const wake of [3601, 5000, Infinity, 'Infinity', '1e400']) { + assert.deepEqual(read(wake), { kind: 'wake', seconds: 3600 }, `wake_in_seconds: ${String(wake)}`); + } + }); + + it('a number below 1 isn\'t a wake request, so 0 can\'t become a wake every second', () => { + for (const wake of [0, 0.5, -5, -Infinity, '0', '-5', '0.999']) { + assert.deepEqual(read(wake), { kind: 'invalid', value: wake }, `wake_in_seconds: ${String(wake)}`); + } + }); + + it('anything else isn\'t a wake request, and keeps the value it got', () => { + for (const wake of ['soon', '10 minutes', '', ' ', 'NaN', Number.NaN, true, false, [5], [], {}]) { + assert.deepEqual(read(wake), { kind: 'invalid', value: wake }, `wake_in_seconds: ${JSON.stringify(wake)}`); + } + }); +}); + +describe('skipReplyWakeRefusal: why a skip_reply call is refused, if it is', () => { + it('a call with no wake is never refused', () => { + assert.equal(skipReplyWakeRefusal({ kind: 'none' }, true), undefined); + assert.equal(skipReplyWakeRefusal({ kind: 'none' }, false), undefined); + }); + + it('with an EventGate, a wake is taken and a value that isn\'t one is refused, named', () => { + assert.equal(skipReplyWakeRefusal({ kind: 'wake', seconds: 1 }, true), undefined); + const refusal = skipReplyWakeRefusal({ kind: 'invalid', value: 'soon' }, true); + assert.equal( + refusal, + 'Your turn didn\'t end, and no wake was set: wake_in_seconds takes a number of seconds from 1 to 3600, and "soon" isn\'t one. Call skip_reply with a number of seconds to come back on your own, or without wake_in_seconds to end the turn and wait for the next message.', + ); + assert.match(skipReplyWakeRefusal({ kind: 'invalid', value: true }, true)!, /, and true isn't one\./); + assert.match(skipReplyWakeRefusal({ kind: 'invalid', value: 0 }, true)!, /, and 0 isn't one\./); + // JSON would write these as null, which names no value the model sent. + assert.match(skipReplyWakeRefusal({ kind: 'invalid', value: -Infinity }, true)!, /, and -Infinity isn't one\./); + assert.match(skipReplyWakeRefusal({ kind: 'invalid', value: Number.NaN }, true)!, /, and NaN isn't one\./); + assert.match(skipReplyWakeRefusal({ kind: 'invalid', value: { at: 'noon' } }, true)!, /, and \{"at":"noon"\} isn't one\./); + }); + + it('names a long value by its first 80 characters, never by half a character', () => { + const refusal = skipReplyWakeRefusal({ kind: 'invalid', value: 'x'.repeat(200) }, true)!; + assert.match(refusal, new RegExp(`and "${'x'.repeat(79)}… isn't one`)); + // 41 emoji are 82 UTF-16 units, so 80 units would end inside the 40th. + const emoji = skipReplyWakeRefusal({ kind: 'invalid', value: '😀'.repeat(41) }, true)!; + assert.ok(emoji.includes(`and "${'😀'.repeat(39)}… isn't one`), emoji); + assert.doesNotMatch(emoji, /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { + assert.equal(skipReplyWakeRefusal({ kind: 'wake', seconds: 60 }, false), SKIP_REPLY_WAKE_NO_GATE_REFUSAL); + assert.equal(skipReplyWakeRefusal({ kind: 'invalid', value: 'soon' }, false), SKIP_REPLY_WAKE_NO_GATE_REFUSAL); + }); +});