fix(mcpl): stopTyping symmetry + immediate typing-metadata refresh

Review findings on anima-research#26:

- stopTyping(channelId) no longer dispatches a 'stop' notification when
  there was no active interval. Matches the global-clear branch's
  semantics and keeps defensive stopTyping(ch) calls from spamming
  stops at a server that never saw a start.
- startTyping(channelId, metadata) on an already-typing channel now
  dispatches an immediate refresh when the new metadata differs from
  the cached one, instead of waiting up to TYPING_INTERVAL_MS for the
  next tick. This is precisely the Zulip topic-switch case the op:
  'stop' commit was meant to cover; the fast-path refresh uses the
  same routing-hint contract without the stop/start flap.

Also adds a comment on the reconnect stderr rebind acknowledging that
it inherits the same "transfer internals, leak listeners on the dead
instance" pattern as the surrounding message-routing and lifecycle
rewiring — a TODO for whoever cleans up that reconnect shape.

Tests: adds test/channel-registry-typing.test.ts (6 cases) covering
both symmetry and the metadata-change fast-path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: Anarchid

src/mcpl/channel-registry.ts

@@ -38,6 +38,21 @@ import type { ToolDefinition, ToolResult, ProcessEvent } from '../types/index.js
 
 const TYPING_INTERVAL_MS = 7_000;
 
+function shallowEqualRecord(
+  a: Record<string, unknown> | undefined,
+  b: Record<string, unknown> | undefined,
+): boolean {
+  if (a === b) return true;
+  if (!a || !b) return false;
+  const keysA = Object.keys(a);
+  const keysB = Object.keys(b);
+  if (keysA.length !== keysB.length) return false;
+  for (const k of keysA) {
+    if (a[k] !== b[k]) return false;
+  }
+  return true;
+}
+
 // ============================================================================
 // Internal Types
 // ============================================================================
@@ -428,12 +443,26 @@ export class ChannelRegistry {
    * No-op if already typing on this channel.
    */
   startTyping(channelId: string, metadata?: Record<string, unknown>): void {
+    const metadataChanged =
+      metadata !== undefined &&
+      !shallowEqualRecord(this.typingMetadata.get(channelId), metadata);
     if (metadata) {
       this.typingMetadata.set(channelId, metadata);
     }
 
     if (this.typingIntervals.has(channelId)) {
-      return; // Already typing — metadata update above still took effect
+      // Already typing. If the caller supplied new routing metadata (e.g. the
+      // relevant Zulip topic just moved because a newer message arrived),
+      // dispatch an immediate refresh so the server sees the new routing
+      // within this request instead of waiting up to TYPING_INTERVAL_MS for
+      // the next tick.
+      if (metadataChanged) {
+        const entry = this.findChannelEntry(channelId);
+        if (entry) {
+          this.sendTypingNotification(entry.serverId, channelId);
+        }
+      }
+      return;
     }
 
     // Find the channel entry and its server
@@ -467,14 +496,16 @@ export class ChannelRegistry {
       if (interval) {
         clearInterval(interval);
         this.typingIntervals.delete(channelId);
-      }
-      // Dispatch an explicit 'stop' so servers that support it (e.g. Zulip)
-      // clear the indicator immediately rather than waiting for auto-expire.
-      // Metadata still carries the routing hint so the stop hits the same
-      // topic/thread as the start.
-      const entry = this.findChannelEntry(channelId);
-      if (entry && this.sendTypingFn) {
-        this.sendTypingFn(entry.serverId, channelId, this.typingMetadata.get(channelId), 'stop');
+        // Dispatch an explicit 'stop' so servers that support it (e.g. Zulip)
+        // clear the indicator immediately rather than waiting for auto-expire.
+        // Metadata still carries the routing hint so the stop hits the same
+        // topic/thread as the start. Guarded by `interval`: matches the
+        // global-clear branch's semantics, and keeps defensive stopTyping(ch)
+        // calls from spamming stops at a server that never saw a start.
+        const entry = this.findChannelEntry(channelId);
+        if (entry && this.sendTypingFn) {
+          this.sendTypingFn(entry.serverId, channelId, this.typingMetadata.get(channelId), 'stop');
+        }
       }
       this.typingMetadata.delete(channelId);
     } else {

src/mcpl/server-connection.ts

@@ -560,6 +560,15 @@ export class McplServerConnection extends EventEmitter {
       this.pendingRequests.clear();
 
       // Re-target stderr forwarding from the throwaway `fresh` instance to `this`.
+      //
+      // Note: this follows the same "transfer internals, leak listeners on the
+      // dead instance" pattern as the message-routing and lifecycle rewiring
+      // below — `fresh`'s own readline 'line' handler and process 'exit'
+      // handler stay registered on the transferred child, short-circuited by
+      // `fresh.closed = true` set in extractInternals(). Rebinding fresh's
+      // stderr listener (rather than removing it) keeps that invariant: if we
+      // ever clean up the message-routing duplication, fresh's stderr data
+      // listener closure-holds onLine and would need the same teardown.
       if (fresh.rebindStderr) {
         fresh.rebindStderr((line: string) => this.emit('stderr', { line }));
         this.rebindStderr = fresh.rebindStderr;

test/channel-registry-typing.test.ts

@@ -0,0 +1,134 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { ChannelRegistry } from '../src/mcpl/channel-registry.js';
+import type { McplServerRegistry } from '../src/mcpl/server-registry.js';
+import type { FeatureSetManager } from '../src/mcpl/feature-set-manager.js';
+
+type TypingCall = {
+  serverId: string;
+  channelId: string;
+  metadata?: Record<string, unknown>;
+  op?: 'start' | 'stop';
+};
+
+function makeRegistry() {
+  const calls: TypingCall[] = [];
+  const sendTypingFn = (
+    serverId: string,
+    channelId: string,
+    metadata?: Record<string, unknown>,
+    op?: 'start' | 'stop',
+  ) => {
+    calls.push({ serverId, channelId, metadata, op });
+  };
+
+  const registry = new ChannelRegistry(
+    {} as McplServerRegistry,
+    {} as FeatureSetManager,
+    () => {},
+    () => {},
+    { sendTypingFn },
+  );
+
+  // Register a channel directly via the private map — avoids the full
+  // channel-incoming event plumbing, which isn't under test here.
+  (registry as unknown as {
+    channels: Map<string, { serverId: string; descriptor: { id: string }; open: boolean }>;
+  }).channels.set('srv:ch1', {
+    serverId: 'srv',
+    descriptor: { id: 'ch1' },
+    open: true,
+  });
+
+  return { registry, calls };
+}
+
+test('stopTyping(channelId) suppresses stop when no interval was active', () => {
+  const { registry, calls } = makeRegistry();
+
+  // Never called startTyping — defensive stop should not dispatch anything.
+  registry.stopTyping('ch1');
+
+  assert.equal(calls.length, 0, 'no typing notifications should be sent when no interval exists');
+});
+
+test('stopTyping(channelId) dispatches stop when an interval was active', () => {
+  const { registry, calls } = makeRegistry();
+
+  registry.startTyping('ch1', { topic: 't1' });
+  assert.equal(calls.length, 1);
+  assert.equal(calls[0].op, undefined); // initial start — op defaults
+  calls.length = 0;
+
+  registry.stopTyping('ch1');
+  assert.equal(calls.length, 1);
+  assert.equal(calls[0].op, 'stop');
+  assert.deepEqual(calls[0].metadata, { topic: 't1' });
+
+  // Second stop on the same channel is a no-op — interval already cleared.
+  registry.stopTyping('ch1');
+  assert.equal(calls.length, 1, 'repeated stopTyping should not re-dispatch');
+});
+
+test('startTyping with changed metadata mid-typing dispatches immediate refresh', () => {
+  const { registry, calls } = makeRegistry();
+
+  registry.startTyping('ch1', { topic: 'old-topic' });
+  assert.equal(calls.length, 1);
+  assert.deepEqual(calls[0].metadata, { topic: 'old-topic' });
+  calls.length = 0;
+
+  // Newer incoming message moves the relevant Zulip topic — caller bumps
+  // startTyping with new routing metadata. Should dispatch immediately rather
+  // than waiting up to 7s for the next tick.
+  registry.startTyping('ch1', { topic: 'new-topic' });
+  assert.equal(calls.length, 1, 'metadata change should trigger immediate notification');
+  assert.deepEqual(calls[0].metadata, { topic: 'new-topic' });
+
+  registry.stopTyping('ch1');
+});
+
+test('startTyping with unchanged metadata mid-typing does not dispatch', () => {
+  const { registry, calls } = makeRegistry();
+
+  registry.startTyping('ch1', { topic: 't' });
+  calls.length = 0;
+
+  registry.startTyping('ch1', { topic: 't' });
+  assert.equal(calls.length, 0, 'no-op when metadata is unchanged');
+
+  registry.stopTyping('ch1');
+});
+
+test('startTyping with no metadata mid-typing does not dispatch', () => {
+  const { registry, calls } = makeRegistry();
+
+  registry.startTyping('ch1', { topic: 't' });
+  calls.length = 0;
+
+  registry.startTyping('ch1');
+  assert.equal(calls.length, 0, 'omitted metadata should not trigger a refresh');
+
+  registry.stopTyping('ch1');
+});
+
+test('stopTyping() global branch only dispatches stops for channels with intervals', () => {
+  const { registry, calls } = makeRegistry();
+
+  // Stash metadata without an interval — simulates a past start that missed
+  // findChannelEntry (pre-registration race).
+  (registry as unknown as { typingMetadata: Map<string, Record<string, unknown>> }).typingMetadata.set(
+    'phantom-ch',
+    { foo: 1 },
+  );
+
+  registry.startTyping('ch1', { topic: 't' });
+  calls.length = 0;
+
+  registry.stopTyping();
+
+  // Exactly one stop for ch1; phantom-ch never had an interval.
+  const stops = calls.filter((c) => c.op === 'stop');
+  assert.equal(stops.length, 1);
+  assert.equal(stops[0].channelId, 'ch1');
+});