diff --git a/changelog.d/user-interrupt-not-failure.fixed.md b/changelog.d/user-interrupt-not-failure.fixed.md new file mode 100644 index 0000000..d81dc3f --- /dev/null +++ b/changelog.d/user-interrupt-not-failure.fixed.md @@ -0,0 +1,15 @@ +- A user-initiated stream stop (host Stop button / `cancelStream()`) is no + longer recorded as an inference failure. It now emits `inference:aborted` + (reason `user`) instead of `inference:exhausted`, leaves the + consecutive-failure streak and ops alerts untouched, and writes an honest + `[turn-interrupted]` chronicle marker ("deliberate cancellation, not a + failure") in place of the misleading `[inference-failed] the model call + failed…` text with remediation advice for a failure that never happened. +- Speech-route failures with no delivery locus (headless/WebUI turns with no + home or trigger channel) now read `[send-undeliverable] … had no delivery + destination` instead of claiming a Discord delivery failure to "the + channel". The machine-readable marker `kind` is unchanged. +- The event gate's inference buffer no longer drops its oldest pending event + silently when full: the drop is logged to stderr with the policy and event + type. A dropped event never triggers inference, so a silent drop looked + like "message sent, never answered, queue depth 0" from the outside. diff --git a/src/framework.ts b/src/framework.ts index 311d884..942e936 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -6872,6 +6872,17 @@ export class AgentFramework { } } const reason = event.reason ?? 'unknown'; + // Membrane emits reason 'user' exactly when someone called + // stream.cancel() — the host's Stop button / Escape / an admin + // abort. That is a DELIBERATE cancellation, not a failure: it + // must not feed the consecutive-failure streak, ops alerts, or + // the "[inference-failed] the model call failed" chronicle + // marker, all of which told the agent (in the user's voice!) + // that its turn failed and advised remediation for a failure + // that never happened. For long-lived residents whose transcript + // is memory, those mislabeled cancellations accumulate as false + // self-knowledge. + const deliberate = reason === 'user'; // Only reset if this is still the active stream (a budget restart // may have already started a new stream, bumping streamId) if (agent.streamId === myStreamId) { @@ -6881,13 +6892,45 @@ export class AgentFramework { this.settleAgent(agent.name, { stopReason: 'exhausted', speech: '', - error: `Stream aborted: ${reason}`, - }); - this.emitTrace({ - type: 'inference:exhausted', - agentName: agent.name, - error: `Stream aborted: ${reason}`, + error: deliberate ? 'Stream stopped by user' : `Stream aborted: ${reason}`, }); + if (deliberate) { + // Distinct trace type: emitTrace funnels every + // inference:exhausted into noteInferenceExhausted (streak, + // failures.log, marker); inference:aborted carries the + // honest cause without any of that. + this.emitTrace({ + type: 'inference:aborted', + agentName: agent.name, + reason: 'user', + durationMs, + }); + // Agent-facing marker, honest about what happened and who + // acted (no inference triggered → no loop). Same system + // envelope as the failure marker so surfaces render it + // the same way. + try { + agent.getContextManager().addMessage( + 'user', + [{ + type: 'text', + text: + `[turn-interrupted] Your previous turn was stopped mid-stream ` + + `by the user — a deliberate cancellation, not a failure. Any ` + + `partial output was cut off by the stop and was not delivered.`, + }], + { system: true, kind: 'turn-interrupted', reason }, + ); + } catch (err) { + console.error(`[turn-interrupted] could not record chronicle marker for ${agent.name}:`, err); + } + } else { + this.emitTrace({ + type: 'inference:exhausted', + agentName: agent.name, + error: `Stream aborted: ${reason}`, + }); + } // Postmortem 2026-05-28 P2 #7: persist the abort to the // inference log so future investigations can attribute the // terminal cause without relying on live in-memory reducer @@ -8904,12 +8947,19 @@ export class AgentFramework { ? `${label.startsWith('#') ? label : `#${label}`} (${channelId})` : channelId : 'the channel'; + // No-locus failures (headless/WebUI turns with no home or + // trigger channel) are not Discord failures: a "[discord-send- + // failed] could not be delivered to the channel" marker sent + // the agent debugging a Discord problem that doesn't exist. + // The machine-readable `kind` stays stable — the gate's + // discord-send-failed-skip intent keys on it — but the text + // the agent reads names the real situation. + const text = channelId + ? `[discord-send-failed] Your previous reply (${textLen} chars) could not be delivered to ${where} (${reason}). It was saved to your archive but the human did not receive it.` + : `[send-undeliverable] Your previous reply (${textLen} chars) had no delivery destination — ${reason}. This is a routing/configuration situation, not a channel failure. The reply was saved to your archive but was not delivered anywhere.`; this.addMessage( 'user', - [{ - type: 'text', - text: `[discord-send-failed] Your previous reply (${textLen} chars) could not be delivered to ${where} (${reason}). It was saved to your archive but the human did not receive it.`, - }], + [{ type: 'text', text }], { system: true, kind: 'discord-send-failed', channelId: channelId ?? '', reason }, ); } catch (err) { diff --git a/src/gate/event-gate.ts b/src/gate/event-gate.ts index a88dbaf..ba9e2a0 100644 --- a/src/gate/event-gate.ts +++ b/src/gate/event-gate.ts @@ -1513,7 +1513,18 @@ export class EventGate { private bufferForInference(events: PendingEvent[]): void { for (const event of events) { if (this.inferenceBuffer.length >= MAX_INFERENCE_BUFFER) { - this.inferenceBuffer.shift(); // Drop oldest + // Never drop silently: a discarded event here is a message that was + // persisted but will never trigger a wake — from the outside it + // looks like "sent, ignored, queue depth 0". If this line ever + // shows up in logs, the buffer is wedged (see flush gate on + // `inferring`) or genuinely overwhelmed; either way the operator + // needs to know which events died. + const dropped = this.inferenceBuffer.shift()!; + console.error( + `[event-gate] inference buffer full (${MAX_INFERENCE_BUFFER}): dropping oldest ` + + `pending event (policy=${dropped.policyName}, type=${dropped.eventType}) to admit ` + + `a newer one. Dropped events never trigger inference.`, + ); } this.inferenceBuffer.push(event); } diff --git a/test/user-interrupt-not-failure.test.ts b/test/user-interrupt-not-failure.test.ts new file mode 100644 index 0000000..ca559d0 --- /dev/null +++ b/test/user-interrupt-not-failure.test.ts @@ -0,0 +1,186 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { + EventResponse, + Module, + ModuleContext, + ProcessEvent, + ProcessState, + ToolCall, + ToolDefinition, + ToolResult, + TraceEvent, +} from '../src/index.js'; +import { AgentFramework } from '../src/index.js'; +import { createMockResponse, MockMembrane } from './helpers/mock-membrane.js'; + +/** + * A user pressing Stop is a deliberate cancellation, not a model failure. + * Before the fix, the host's cancelStream() path fell through driveStream's + * generic abort handling into the failure pipeline: an inference:exhausted + * trace, a bumped consecutive-failure streak (three Stops → hard-down ops + * alert), and an "[inference-failed] the model call failed and produced no + * response … drop an oversized attachment" chronicle marker attributed to + * the user — three inaccuracies (wrong cause, wrong speaker, irrelevant + * advice) accumulating as false self-knowledge in a resident's transcript. + */ + +/** Module whose tool call hangs until released — keeps the stream open so + * the test can cancel mid-turn, exactly as the TUI/WebUI Stop button does. */ +class HangingToolModule implements Module { + readonly name = 'test'; + release!: () => void; + private readonly gate = new Promise((resolve) => { this.release = resolve; }); + + async start(_ctx: ModuleContext): Promise {} + async stop(): Promise {} + + getTools(): ToolDefinition[] { + return [{ + name: 'hang', + description: 'Hangs until released', + inputSchema: { type: 'object', properties: {} }, + }]; + } + + async handleToolCall(_call: ToolCall): Promise { + await this.gate; + return { success: true, data: {} }; + } + + async onProcess(event: ProcessEvent, _state: ProcessState): Promise { + if (event.type === 'external-message') { + return { + addMessages: [{ participant: 'User', content: [{ type: 'text', text: String(event.content) }] }], + requestInference: true, + }; + } + return {}; + } +} + +async function waitFor(cond: () => boolean, ms = 2000): Promise { + const start = Date.now(); + while (!cond()) { + if (Date.now() - start > ms) throw new Error('timeout waiting for condition'); + await new Promise((r) => setTimeout(r, 10)); + } +} + +describe('user interrupt is not recorded as a failure', () => { + it('cancelStream mid-turn → [turn-interrupted] marker, inference:aborted trace, no failure streak', async () => { + const tempDir = mkdtempSync(join(tmpdir(), 'interrupt-')); + const membrane = new MockMembrane(); + // One response with a hanging tool call: the stream stays alive in + // waiting_for_tools until we cancel it. + membrane.pushResponse(createMockResponse( + [{ type: 'tool_use', id: 't1', name: 'test--hang', input: {} } as never], + 'tool_use', + )); + + const module = new HangingToolModule(); + const framework = await AgentFramework.create({ + storePath: join(tempDir, 'test.chronicle'), + membrane: membrane.asMembrane(), + agents: [{ name: 'assistant', model: 'test-model', systemPrompt: 'Assist.' }], + modules: [module], + }); + + const traces: TraceEvent[] = []; + framework.onTrace((t) => { traces.push(t); }); + + try { + framework.pushEvent({ type: 'external-message', source: 'test', content: 'go', metadata: {} }); + framework.start(); + + const agent = framework.getAgent('assistant')!; + await waitFor(() => agent.state.status === 'waiting_for_tools'); + + // What the TUI / WebUI Stop button does. + agent.cancelStream(); + + await waitFor(() => traces.some((t) => t.type === 'inference:aborted')); + // Let the abort settle fully (chronicle marker write). + await waitFor(() => { + const { messages } = agent.getContextManager().queryMessages({}); + return messages.some((m) => + m.content.some((b) => b.type === 'text' && b.text.includes('[turn-interrupted]'))); + }); + + const { messages } = agent.getContextManager().queryMessages({}); + const texts = messages.flatMap((m) => + m.content.filter((b): b is { type: 'text'; text: string } => b.type === 'text').map((b) => b.text)); + + // Honest marker present… + const marker = texts.find((t) => t.includes('[turn-interrupted]')); + assert.ok(marker, 'expected a [turn-interrupted] chronicle marker'); + assert.match(marker!, /deliberate cancellation, not a failure/); + // …and no failure framing anywhere. + assert.ok(!texts.some((t) => t.includes('[inference-failed]')), + 'a user stop must not produce an [inference-failed] marker'); + + // Trace: aborted (with the honest reason), not exhausted. + const aborted = traces.find((t) => t.type === 'inference:aborted') as { reason?: string }; + assert.equal(aborted?.reason, 'user'); + assert.ok(!traces.some((t) => t.type === 'inference:exhausted'), + 'a user stop must not emit inference:exhausted (feeds streak + ops alerts)'); + } finally { + // Release the hung tool BEFORE stopping: its completion pushes a + // tool-result event, which must land while the queue is still open. + module.release(); + await new Promise((r) => setTimeout(r, 50)); + await framework.stop(); + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it('a real provider abort (non-user reason) still goes through the failure pipeline', async () => { + const tempDir = mkdtempSync(join(tmpdir(), 'interrupt-real-')); + const membrane = new MockMembrane(); + membrane.pushResponse(createMockResponse( + [{ type: 'tool_use', id: 't1', name: 'test--hang', input: {} } as never], + 'tool_use', + )); + + const module = new HangingToolModule(); + const framework = await AgentFramework.create({ + storePath: join(tempDir, 'test.chronicle'), + membrane: membrane.asMembrane(), + agents: [{ name: 'assistant', model: 'test-model', systemPrompt: 'Assist.' }], + modules: [module], + }); + + const traces: TraceEvent[] = []; + framework.onTrace((t) => { traces.push(t); }); + + try { + framework.pushEvent({ type: 'external-message', source: 'test', content: 'go', metadata: {} }); + framework.start(); + + const agent = framework.getAgent('assistant')!; + await waitFor(() => agent.state.status === 'waiting_for_tools'); + + // Simulate a provider-side abort: emit the event with a non-user + // reason directly on the live mock stream. + const stream = membrane.lastStream!; + (stream as unknown as { events: unknown[]; pendingResolve: (() => void) | null }).events.push( + { type: 'aborted', reason: 'connection_lost' }); + const pr = (stream as unknown as { pendingResolve: (() => void) | null }).pendingResolve; + if (pr) { (stream as unknown as { pendingResolve: null }).pendingResolve = null; pr(); } + + await waitFor(() => traces.some((t) => t.type === 'inference:exhausted')); + const exhausted = traces.find((t) => t.type === 'inference:exhausted') as { error?: string }; + assert.match(exhausted?.error ?? '', /connection_lost/); + } finally { + // Release the hung tool BEFORE stopping: its completion pushes a + // tool-result event, which must land while the queue is still open. + module.release(); + await new Promise((r) => setTimeout(r, 50)); + await framework.stop(); + rmSync(tempDir, { recursive: true, force: true }); + } + }); +});