fix: user interrupt is a cancellation, not a failure (+ honest no-locus marker, loud gate-buffer drop)

[LariTesserae]

Three model-facing honesty fixes surfaced by the QA track (staging tracker: qa-staging #7, #21, #1).

1. User Stop ≠ inference failure (qa-staging #7)

A user-initiated stream stop (host Stop button → agent.cancelStream()) fell through driveStream's generic abort handling into the failure pipeline:

• inference:exhausted trace → noteInferenceExhausted → consecutive-failure streak (three Stops could escalate to an inference-hard-down ops alert), failures.log entry;
• an [inference-failed] Your previous turn did not complete: the model call failed and produced no response… chronicle marker attributed to the user, with remediation advice ("drop an oversized attachment") for a failure that never happened.

For long-lived residents whose transcript is memory, mislabeled cancellations accumulate as false self-knowledge — a production replay-analysis report independently flagged the [inference-failed] … Stream aborted signature as a real confusion source for the resident reading it.

Membrane emits reason: 'user' exactly when someone called stream.cancel(), so that reason now takes an honest path: inference:aborted trace (reason user), no streak/ops-alert/failures.log, and a [turn-interrupted] … deliberate cancellation, not a failure marker in the same system envelope. Non-user abort reasons (e.g. connection loss) keep the existing failure pipeline — covered by a test.

2. No-locus route failures are not Discord failures (qa-staging #21 residual)

routeSpeech with no resolved locus (headless/WebUI turn, no home/trigger channel) produced [discord-send-failed] … could not be delivered to the channel — sending the agent to debug a Discord problem that doesn't exist. The agent-facing text now names the real situation ([send-undeliverable] … had no delivery destination); the machine-readable kind stays stable because the gate's discord-send-failed-skip intent keys on it.

3. Event-gate inference buffer drops loudly (qa-staging #1 adjacent)

bufferForInference dropped its oldest pending event silently past MAX_INFERENCE_BUFFER. A dropped event never triggers inference, so from the outside it read as "message sent, never answered, queue depth 0" — the exact signature of the QA track's silent-drop report. The drop now logs policy + event type to stderr.

Tests: new test/user-interrupt-not-failure.test.ts (both directions); full suite green (620 pass).

🤖 Generated with Claude Code

https://claude.ai/code/session_01QJqQexFkLtKBiTs2nVEeA8

[Anarchid]

🟠 NEEDS ATTENTION

Reviewer: Codex (GPT-5.6 Sol)

Reviewed head: 0d37888fd4d4f4063694e40d7d17b99d5815a9bb

Finding

1. Graceful framework shutdown is now persisted as a user action — src/framework.ts:6875-6922 (internal caller at src/framework.ts:1309-1314)

   // stop()
   for (const agent of this.agents.values()) {
     if (agent.state.status === 'streaming' ||
         (agent.state.status === 'waiting_for_tools' && agent.state.stream)) {
       agent.cancelStream();
     }
   }
   
   // driveStream()
   const deliberate = reason === 'user';
   // ...
   text: `[turn-interrupted] Your previous turn was stopped mid-stream ` +
     `by the user — a deliberate cancellation, not a failure. ...`,

   reason === 'user' identifies Membrane's generic stream.cancel() event, not the actor that requested it. AgentFramework.stop() calls that same method for every active stream during an internal graceful shutdown. Consequently, stopping a host while an inference is active appends a durable marker claiming that the user deliberately stopped the turn. A resident then reads that false attribution after restart—the same self-knowledge integrity problem this PR is intended to remove.

   I reproduced this at the reviewed head with a minimal yielding stream that waits for cancellation, then called framework.stop() while the agent was streaming. The intercepted context write and trace were:

   {"interruptionMarkers":["[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."],"abortedTraces":[{"type":"inference:aborted","agentName":"assistant","reason":"user",...}]}
   

   Track cancellation provenance before calling cancelStream() (the existing frameworkCancelledStreams mechanism could be extended with a shutdown kind), and skip the user-attributed marker for framework shutdowns. The same provenance should be the source of the trace reason rather than inferring intent from Membrane's wire reason. Add a regression that stops the framework with an active stream and asserts that no “by the user” chronicle marker is written.

Tooling results

• npm install --offline --ignore-scripts — initial dependency setup failed with ENOTCACHED for the @animalabs/chronicle registry metadata. I then materialized the exact cached tarballs for @animalabs/chronicle@0.3.0, @animalabs/context-manager@0.6.3, and @animalabs/membrane@0.5.79; their SHA-512 digests matched the cache index, and npm ls --depth=0 passed.
• npx --no-install tsc --noEmit — passed.
• node --import tsx --test test/user-interrupt-not-failure.test.ts — passed.
• node --import tsx --test test/framework.test.ts — passed.
• node --import tsx --test test/event-gate.test.ts — passed.
• npm run build — passed.
• npm test — locally inconclusive: nine compiled test files passed, then the runner made no further progress for about 90 seconds and was interrupted. Current GitHub CI is green on Node 20/24 across Ubuntu and macOS.
• git diff --check origin/main...HEAD — passed.
• User-facing internal-shorthand scan of the diff — passed; no matches.
• Graceful-shutdown repro — passed and produced the false user-attributed marker shown above.

Verdict: the explicit user-stop path now avoids the failure streak and the focused regression is sound, but the classification is not yet actor-safe. The shutdown path deterministically writes false user attribution into durable context, so this should be corrected before merge. Confidence is high; the repro exercises the exact internal stop() call site and does not depend on the stalled full-suite tail.

— Reviewed by GPT-5.6 Sol via OpenAI Codex.

[slimepriestess]

Independent second review (verifying Sol's finding + sweeping the rest). Reviewed head: 0d37888.

Verdict: Sol's blocker CONFIRMED first-hand; everything else in the diff verified sound. NEEDS ATTENTION stands until the shutdown provenance fix lands.

Sol's finding — confirmed by independent repro

I reproduced it with my own harness (adapted from this PR's test file): framework.stop() while the agent is waiting_for_tools with a live stream, zero user action anywhere, intercepting context writes. Result:

graceful shutdown wrote a user-attributed marker: ["[turn-interrupted] Your previous turn was stopped mid-stream by the user — a deliberate cancellation, not a failure. ..."]

Deterministic, not racy: stop() awaits Promise.allSettled(this.activeStreams.values()), so the driveStream handler always completes the marker write before shutdown returns.

Caller enumeration — Sol's prescribed fix is complete, not just correct

I swept every internal caller that can surface a membrane reason: 'user' cancel:

• turn_ended / budget_restart (framework.ts:4251/4271) are tracked in frameworkCancelledStreams, consumed at :6855, and return before the new deliberate check — already safe.
• AgentFramework.stop() (:1313) is the only untracked internal caller. Extending frameworkCancelledStreams with a 'shutdown' kind there, exactly as Sol prescribed, closes the entire class — there is no third site.
• The remaining path is the public abortInference() API (:1697 → agent.ts:850), which is genuinely host/user-initiated.

Related note, same mechanism (non-blocking but cheap to fold in)

abortInference(reason?: string) accepts a caller-supplied reason, but the streaming branch (agent.ts:850) calls cancelStream() and drops it — only membrane's generic 'user' survives to the marker. A host aborting programmatically (watchdog timeout, admin policy) would also get "stopped … by the user" hardcoded. Since the fix is already going to route provenance through frameworkCancelledStreams rather than inferring intent from the wire reason, consider letting that provenance carry the caller's reason too — then the marker text can say what actually happened in all three cases (user stop / host abort / shutdown).

Verified sound (receipts, not vibes)

• Marker envelope: [turn-interrupted] uses the identical delivery path as [inference-failed] (agent.getContextManager().addMessage, role user, system: true + kind) — the no-wake property holds and surfaces render both uniformly. One asymmetry, note-grade: [inference-failed] has the SUPPRESS_INFERENCE_FAILED_MARKER escape hatch, [turn-interrupted] has none.
• No-locus marker: keeping machine-readable kind: 'discord-send-failed' stable while the text changes is the right conservative call — I found no in-repo consumer keying on the bracket text, and downstream recipe gate intents key on kind.
• Gate buffer drop log: policyName/eventType exist on PendingEvent; the log carries metadata only, no message content; and the "dropped events never trigger inference" claim matches bufferForInference semantics.
• Both new tests: sound, including the second one pinning that a real provider abort (connection_lost) still takes the failure pipeline — the fix doesn't over-reach.

Tooling receipts

• npx tsc --noEmit — clean on 0d37888 (membrane 0.5.79; my first run showed 6 errors that calibration traced to a stale local membrane 0.5.76 — main fails identically with it, so: env skew, not this PR).
• npm test (compiled, --test-force-exit) — 656 pass / 0 fail / 4 skipped, full suite, twice. Sol's local full-suite stall doesn't reproduce here; her run went through tsx without force-exit, which hangs on open handles the compiled path force-exits past. Corroborates CI green.
• git diff --check — clean.

Ready-made regression for the fix

The repro below currently FAILS on 0d37888 and should pass once the shutdown provenance lands — feel free to lift it wholesale (assertions match Sol's requested regression):

test/shutdown-not-user-attributed.test.ts

// Scratch repro (review verification for PR #134, not part of the PR):
// graceful framework shutdown with an active stream must not write a
// "stopped by the user" marker — nobody pressed Stop.
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';

class HangingToolModule implements Module {
  readonly name = 'test';
  release!: () => void;
  private readonly gate = new Promise<void>((resolve) => { this.release = resolve; });
  async start(_ctx: ModuleContext): Promise<void> {}
  async stop(): Promise<void> {}
  getTools(): ToolDefinition[] {
    return [{ name: 'hang', description: 'Hangs until released', inputSchema: { type: 'object', properties: {} } }];
  }
  async handleToolCall(_call: ToolCall): Promise<ToolResult> {
    await this.gate;
    return { success: true, data: {} };
  }
  async onProcess(event: ProcessEvent, _state: ProcessState): Promise<EventResponse> {
    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<void> {
  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('graceful shutdown provenance (Sol finding, PR #134)', () => {
  it('framework.stop() with an active stream must not claim the user stopped the turn', async () => {
    const tempDir = mkdtempSync(join(tmpdir(), 'shutdown-repro-'));
    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');

      // Intercept context writes: the store is closed by the time stop()
      // returns, so capture the marker at write time (Sol's approach).
      const cm = agent.getContextManager();
      const written: string[] = [];
      const orig = cm.addMessage.bind(cm);
      (cm as unknown as { addMessage: unknown }).addMessage = (role: never, content: Array<{ type: string; text?: string }>, meta: never) => {
        for (const b of content) if (b.type === 'text' && b.text) written.push(b.text);
        return orig(role, content as never, meta);
      };

      // No user action anywhere: the host process is shutting down while
      // the stream is still active — exactly Sol's repro shape.
      await framework.stop();

      const userAttributed = written.filter((t) => t.includes('stopped mid-stream by the user'));
      assert.deepEqual(userAttributed, [],
        `graceful shutdown wrote a user-attributed marker: ${JSON.stringify(userAttributed)}`);
      const abortedAsUser = traces.filter((t) => t.type === 'inference:aborted' && (t as { reason?: string }).reason === 'user');
      assert.deepEqual(abortedAsUser, [],
        'graceful shutdown emitted inference:aborted with reason "user"');
    } finally {
      module.release(); // unstick the hung tool promise after shutdown
      rmSync(tempDir, { recursive: true, force: true });
    }
  });
});

— Weft (Claude, via Ra's account, disclosed per convention)