🔊 fix: warn when summarization.trigger.type is unrecognized (#108)

* 🔊 fix: warn when `trigger.type` is unrecognized

`shouldTriggerSummarization` silently returned `false` whenever the
configured `trigger.type` was not one of the three it implements
(`token_ratio`, `remaining_tokens`, `messages_to_refine`). That turned
any upstream schema/SDK drift into a silent no-op — summarization just
never fired, with no signal to the operator.

Emit a one-time `console.warn` per unrecognized type so the
misconfiguration surfaces in logs without spamming every agent turn.

Behavior is unchanged for all valid trigger types and for the
no-trigger-configured default.

* 🧹 fix: bound the unrecognized-trigger dedup set

Per Codex review: the dedup `Set` for warned trigger types was
process-global and grew unboundedly. Consumers that thread dynamic or
user-provided values through `trigger.type` could accumulate unique
keys in long-lived workers — a minor but real memory vector.

Cap the set at 32 entries and evict the oldest on overflow (Set
preserves insertion order, so this is FIFO-ish). 32 is well above the
handful of typos a legitimate deployment would ever see, and the dedup
contract still holds: we never warn twice for a type that is currently
in the set.

Author: danny-avila

src/summarization/__tests__/trigger.test.ts

@@ -1,4 +1,7 @@
-import { shouldTriggerSummarization } from '@/summarization';
+import {
+  shouldTriggerSummarization,
+  _resetUnrecognizedTriggerWarnings,
+} from '@/summarization';
 
 describe('shouldTriggerSummarization', () => {
   it('uses pre-prune pressure for token_ratio triggers when messages were pruned', () => {
@@ -47,4 +50,100 @@ describe('shouldTriggerSummarization', () => {
 
     expect(result).toBe(false);
   });
+
+  describe('unrecognized trigger type', () => {
+    let warnSpy: jest.SpyInstance;
+
+    beforeEach(() => {
+      _resetUnrecognizedTriggerWarnings();
+      warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined);
+    });
+
+    afterEach(() => {
+      warnSpy.mockRestore();
+    });
+
+    it('does not fire and warns once per unrecognized type', () => {
+      const baseParams = {
+        maxContextTokens: 2500,
+        prePruneContextTokens: 2400,
+        remainingContextTokens: 100,
+        messagesToRefineCount: 4,
+      };
+
+      // Cast via `unknown` because the type union guards against this at compile
+      // time; we are intentionally exercising the runtime fallback.
+      const result1 = shouldTriggerSummarization({
+        ...baseParams,
+        trigger: { type: 'token_count', value: 8000 } as unknown as {
+          type: 'token_ratio';
+          value: number;
+        },
+      });
+
+      expect(result1).toBe(false);
+      expect(warnSpy).toHaveBeenCalledTimes(1);
+      expect(warnSpy.mock.calls[0][0]).toContain('token_count');
+      expect(warnSpy.mock.calls[0][0]).toContain('token_ratio');
+      expect(warnSpy.mock.calls[0][0]).toContain('remaining_tokens');
+      expect(warnSpy.mock.calls[0][0]).toContain('messages_to_refine');
+
+      // Same unrecognized type a second time: no duplicate warning.
+      shouldTriggerSummarization({
+        ...baseParams,
+        trigger: { type: 'token_count', value: 8000 } as unknown as {
+          type: 'token_ratio';
+          value: number;
+        },
+      });
+      expect(warnSpy).toHaveBeenCalledTimes(1);
+
+      // Different unrecognized type: warns again, once.
+      shouldTriggerSummarization({
+        ...baseParams,
+        trigger: { type: 'nonsense', value: 1 } as unknown as {
+          type: 'token_ratio';
+          value: number;
+        },
+      });
+      expect(warnSpy).toHaveBeenCalledTimes(2);
+      expect(warnSpy.mock.calls[1][0]).toContain('nonsense');
+    });
+
+    it('does not grow memory unboundedly under a flood of unique types', () => {
+      const baseParams = {
+        maxContextTokens: 2500,
+        prePruneContextTokens: 2400,
+        remainingContextTokens: 100,
+        messagesToRefineCount: 4,
+      };
+
+      for (let i = 0; i < 500; i++) {
+        shouldTriggerSummarization({
+          ...baseParams,
+          trigger: { type: `bogus-${i}`, value: 1 } as unknown as {
+            type: 'token_ratio';
+            value: number;
+          },
+        });
+      }
+
+      // Still logged each new type (up to the cap) — we never silently dropped
+      // warnings; we just evicted oldest entries from the dedup set.
+      expect(warnSpy).toHaveBeenCalledTimes(500);
+
+      // Re-warns for a recently-seen type that should still be in the cache
+      // (last one just inserted). No duplicate warning means the dedup set
+      // still functions; the size cap did not break the dedup contract.
+      const beforeRecent = warnSpy.mock.calls.length;
+      shouldTriggerSummarization({
+        ...baseParams,
+        trigger: { type: 'bogus-499', value: 1 } as unknown as {
+          type: 'token_ratio';
+          value: number;
+        },
+      });
+      expect(warnSpy).toHaveBeenCalledTimes(beforeRecent);
+    });
+  });
 });

src/summarization/index.ts

@@ -1,5 +1,51 @@
 import type { SummarizationTrigger } from '@/types';
 
+const VALID_TRIGGER_TYPES = [
+  'token_ratio',
+  'remaining_tokens',
+  'messages_to_refine',
+] as const;
+
+/**
+ * Upper bound on the dedup set for unrecognized trigger types. Bounds memory in
+ * case a caller threads dynamic/user-provided strings through `trigger.type`.
+ * Well above the handful of legit misconfigurations a process would ever see.
+ */
+const MAX_WARNED_TRIGGER_TYPES = 32;
+
+const warnedUnrecognizedTriggerTypes = new Set<string>();
+
+/**
+ * Warn (once per process, per unrecognized type) when the configured trigger
+ * type is something the runtime does not evaluate. Without this, a misconfigured
+ * `trigger.type` silently disables summarization with no visible signal.
+ *
+ * The dedup set is size-capped; on overflow we evict the oldest entry (Set
+ * preserves insertion order) so we keep bounded memory and still warn on
+ * recently-seen types.
+ */
+function warnUnrecognizedTriggerType(type: string): void {
+  if (warnedUnrecognizedTriggerTypes.has(type)) {
+    return;
+  }
+  if (warnedUnrecognizedTriggerTypes.size >= MAX_WARNED_TRIGGER_TYPES) {
+    const oldest = warnedUnrecognizedTriggerTypes.values().next().value;
+    if (oldest !== undefined) {
+      warnedUnrecognizedTriggerTypes.delete(oldest);
+    }
+  }
+  warnedUnrecognizedTriggerTypes.add(type);
+  console.warn(
+    `[shouldTriggerSummarization] Unrecognized trigger.type: "${type}". ` +
+      `Summarization will not fire. Valid values: ${VALID_TRIGGER_TYPES.join(', ')}.`
+  );
+}
+
+/** For tests only. Resets the dedup set so warnings can be observed again. */
+export function _resetUnrecognizedTriggerWarnings(): void {
+  warnedUnrecognizedTriggerTypes.clear();
+}
+
 /**
  * Determines whether summarization should be triggered based on the configured trigger
  * and current context state.
@@ -98,5 +144,6 @@ export function shouldTriggerSummarization(params: {
   }
 
   // Unrecognized trigger type: cannot evaluate, do not fire.
+  warnUnrecognizedTriggerType(trigger.type);
   return false;
 }