From 860237da784dd026be7c5f80b5af402c40d9ded5 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Sun, 7 Jun 2026 17:48:07 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=8D=20fix:=20silent-mode=20collector?= =?UTF-8?q?=20+=20real=20layering=20test=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - messagePiiFilter.ts: skip collector.matches.push in silent mode (no consumer reads it; small but pointless allocation per match). Also unifies the redaction log line so silent and warn both record what matched, just at debug-ish info level. Silent's contract is "no UI feedback," not "no server-side observability." - messagePiiFilter.spec.ts: the previous "layers customPatterns on top of starters" test passed starterPatterns: [], which meant only the custom pattern ever fired; the test name lied. Renamed to "uses customPatterns only when starterPatterns is empty" and added a real layering test that exercises both a starter (sk_prefix) and a custom (acme) hitting the same prompt; asserts both ids land in collector.matches and both prefixes appear in the rewritten prompt. - Silent-mode test updated to assert collector.matches is empty (was asserting length 1; corrected to match the new behavior). --- .../agents/__tests__/messagePiiFilter.spec.ts | 25 ++++++++++++++++--- packages/api/src/agents/messagePiiFilter.ts | 16 ++++++------ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/api/src/agents/__tests__/messagePiiFilter.spec.ts b/packages/api/src/agents/__tests__/messagePiiFilter.spec.ts index dadecde599..d8a6ae7030 100644 --- a/packages/api/src/agents/__tests__/messagePiiFilter.spec.ts +++ b/packages/api/src/agents/__tests__/messagePiiFilter.spec.ts @@ -47,7 +47,8 @@ describe('createMessagePiiFilterHooks', () => { expect(result.updatedPrompt).toBe('key sk-[REDACTED] please'); expect(result.decision).toBeUndefined(); - expect(built!.collector.matches).toHaveLength(1); + // Silent mode skips collector population (no consumer reads it) + expect(built!.collector.matches).toEqual([]); }); it('is a no-op when nothing matches', async () => { @@ -110,7 +111,7 @@ describe('createMessagePiiFilterHooks', () => { expect(result.updatedPrompt).toBe('auth Bearer [REDACTED] and key sk-ant-1234567890ABC'); }); - it('layers customPatterns on top of starters', async () => { + it('uses customPatterns only when starterPatterns is empty', async () => { const built = createMessagePiiFilterHooks( silent({ starterPatterns: [], @@ -123,7 +124,25 @@ describe('createMessagePiiFilterHooks', () => { }); expect(result.updatedPrompt).toBe('token [REDACTED] ok'); - expect(built!.collector.matches.map((m) => m.patternId)).toEqual(['acme']); + }); + + it('layers customPatterns on top of starters (both fire in one prompt)', async () => { + const built = createMessagePiiFilterHooks( + silent({ + onMatch: 'warn', + customPatterns: [{ id: 'acme', label: 'Acme token', regex: '\\bACME-[A-Z0-9]{6,}' }], + }), + ); + const result = await executeHooks({ + registry: built!.registry, + input: promptInput('starter sk-anything-here and custom ACME-DEADBEEF12'), + }); + + const ids = built!.collector.matches.map((m) => m.patternId).sort(); + expect(ids).toContain('sk_prefix'); + expect(ids).toContain('acme'); + expect(result.updatedPrompt).toContain('sk-[REDACTED]'); + expect(result.updatedPrompt).toContain('[REDACTED]'); }); it('honors a custom redactionText', async () => { diff --git a/packages/api/src/agents/messagePiiFilter.ts b/packages/api/src/agents/messagePiiFilter.ts index 9879c4e3fd..71919038c7 100644 --- a/packages/api/src/agents/messagePiiFilter.ts +++ b/packages/api/src/agents/messagePiiFilter.ts @@ -91,7 +91,9 @@ export function createMessagePiiFilterHooks( return {}; } - collector.matches.push(...matches); + if (mode !== 'silent') { + collector.matches.push(...matches); + } if (mode === 'block') { logger.info( @@ -108,13 +110,11 @@ export function createMessagePiiFilterHooks( // silent + warn both redact server-side. The difference is // that warn surfaces the matches to the UI via the controller // (which reads collector.matches after processStream resolves). - if (mode === 'warn') { - logger.info( - `[messagePiiFilter] redacted ${matches.length} match(es) (mode=warn, patterns=${matches - .map((m) => m.patternId) - .join(',')})`, - ); - } + logger.info( + `[messagePiiFilter] redacted ${matches.length} match(es) (mode=${mode}, patterns=${matches + .map((m) => m.patternId) + .join(',')})`, + ); return { updatedPrompt: text }; },