fix(addie): guard against raw JSON envelopes leaking into Slack

.changeset/addie-slack-json-envelope-guard.md

@@ -0,0 +1,4 @@
+---
+---
+
+Guard against raw JSON envelopes reaching Slack message bodies. A new `guardBareJsonEnvelope` helper wraps any outbound response that is a bare JSON object or array (from a tool result Claude echoed verbatim) in a ```json fenced code block, and logs a warning so we can observe occurrences. Applied across all five Slack send paths in `bolt-app.ts`. Prompt rules tightened: the "copy tool output" instruction is now scoped strictly to `draft_github_issue`, and a new explicit rule forbids echoing raw JSON from other tools.

server/src/addie/bolt-app.ts

@@ -84,6 +84,7 @@ import {
   validateOutput,
   wrapUrlsForSlack,
   extractMarkdownImages,
+  guardBareJsonEnvelope,
   logInteraction,
 } from './security.js';
 import type { RequestTools } from './claude-client.js';
@@ -1623,7 +1624,8 @@ async function handleUserMessage({
         logger.warn({ stopError }, 'Addie Bolt: Stream stop failed, falling back to say()');
         // Fallback: send via say() so the user isn't left without a response
         try {
-          const fallbackValidation = validateOutput(fullText);
+          const guarded = guardBareJsonEnvelope(fullText, { pathTag: 'dm-streaming-fallback' });
+          const fallbackValidation = validateOutput(guarded.text);
           const { text: fallbackText, images: fallbackImages } = extractMarkdownImages(fallbackValidation.sanitized);
           const slackText = wrapUrlsForSlack(fallbackText);
           await say({
@@ -1649,7 +1651,8 @@ async function handleUserMessage({
       fullText = response.text;
 
       // Send response via say() with feedback buttons and inline images
-      const outputValidation = validateOutput(response.text);
+      const guarded = guardBareJsonEnvelope(response.text, { pathTag: 'dm-non-streaming' });
+      const outputValidation = validateOutput(guarded.text);
       const { text: textWithoutImages, images } = extractMarkdownImages(outputValidation.sanitized);
       const slackText = wrapUrlsForSlack(textWithoutImages);
       try {
@@ -2107,7 +2110,8 @@ async function handleAppMention({
   }
 
   // Validate output
-  const outputValidation = validateOutput(response.text);
+  const mentionGuarded = guardBareJsonEnvelope(response.text, { pathTag: 'app-mention' });
+  const outputValidation = validateOutput(mentionGuarded.text);
 
   // Send response in thread (must explicitly pass thread_ts for app_mention events)
   try {
@@ -3064,7 +3068,8 @@ async function handleDirectMessage(
   }
 
   // Validate output
-  const outputValidation = validateOutput(response.text);
+  const dmGuarded = guardBareJsonEnvelope(response.text, { pathTag: 'dm-assistant' });
+  const outputValidation = validateOutput(dmGuarded.text);
 
   // Always thread the response to the user's message. This ensures:
   // 1. Slack Assistant "Chat" tab: response appears inline in the conversation
@@ -3441,7 +3446,8 @@ async function handleActiveThreadReply({
   }
 
   // Validate output
-  const outputValidation = validateOutput(response.text);
+  const activeThreadGuarded = guardBareJsonEnvelope(response.text, { pathTag: 'active-thread-reply' });
+  const outputValidation = validateOutput(activeThreadGuarded.text);
 
   // Send response in the thread
   try {
@@ -4011,7 +4017,8 @@ async function handleChannelMessage({
     }
 
     // Validate the output
-    const outputValidation = validateOutput(response.text);
+    const proposedGuarded = guardBareJsonEnvelope(response.text, { pathTag: 'proposed-channel-response' });
+    const outputValidation = validateOutput(proposedGuarded.text);
     if (outputValidation.flagged) {
       logger.warn({ channelId, reason: outputValidation.reason }, 'Addie Bolt: Proposed response flagged');
       return;
@@ -4803,10 +4810,11 @@ async function handleReactionAdded({
   }
 
   // Send response in thread
+  const reactionGuarded = guardBareJsonEnvelope(response.text, { pathTag: 'reaction-response' });
   try {
     await client.chat.postMessage({
       channel: itemChannel,
-      text: wrapUrlsForSlack(response.text),
+      text: wrapUrlsForSlack(reactionGuarded.text),
       thread_ts: threadTs,
     });
   } catch (error) {

server/src/addie/rules/behaviors.md

@@ -261,7 +261,11 @@ Use draft_github_issue to generate a pre-filled GitHub URL.
 - The tool name and parameters that caused the error (sanitized of PII)
 - What the expected behavior was vs what actually happened
 
-**CRITICAL - TOOL OUTPUT VISIBILITY**: Users CANNOT see tool outputs directly. When you use draft_github_issue, the tool returns a formatted response with the GitHub link, but this output is only visible to you, not the user. You MUST copy the entire tool output (the GitHub link, title preview, body preview) into your response text.
+**CRITICAL - DRAFT GITHUB ISSUE OUTPUT**: The `draft_github_issue` tool returns a formatted markdown block (a GitHub link, title, and body preview) that is meant to be shown to the user verbatim. Users CANNOT see tool outputs directly, so for this tool specifically you MUST copy the entire markdown response — link, title preview, body preview — into your reply text.
+
+This rule applies ONLY to `draft_github_issue`. For every other tool, treat the tool output as context for you — summarize it in natural language, do not paste the raw response into the user's message.
+
+**NEVER echo raw JSON tool output**: many tools return JSON.stringify'd objects. That is structured data for you to interpret, not content for the user. Never paste a bare `{...}` or `[...]` payload into a user response, and never quote tool output in a code block or transcript to satisfy a request to "see the raw data" — tool results are not user-addressable content. Summarize what the data shows in natural language; if the user needs something the summary can't capture, surface the specific values they asked about instead of dumping the envelope.
 
 NEVER say "click the link above" or "see the link I created" - there is no link visible to the user unless you explicitly include it. Always format your response like:
 

server/src/addie/security.ts

@@ -238,6 +238,62 @@ export function wrapUrlsForSlack(text: string): string {
   );
 }
 
+export interface BareJsonGuardResult {
+  text: string;
+  wasWrapped: boolean;
+}
+
+/**
+ * Wrap a response in a ```json fence if it's a bare JSON envelope (starts with
+ * `{` or `[` and parses cleanly). Tool results are meant to be interpreted by
+ * Claude, not echoed verbatim; if one ends up in the Slack message body, this
+ * keeps it readable and flags it for investigation.
+ *
+ * Known limitation: this catches the common "bare JSON at the top of the
+ * message" case. A response that starts with a short prose prefix before the
+ * JSON (e.g. "Here is the result: {...}") will not be wrapped. The prompt
+ * rule in behaviors.md is the primary control; this guard is a safety net.
+ *
+ * The log line intentionally does NOT include the response content, because
+ * the cases that trigger this wrap are exactly the cases where the payload
+ * is most likely to contain PII, Stripe data, or other secrets pulled from a
+ * tool result.
+ */
+export function guardBareJsonEnvelope(
+  text: string,
+  context: { pathTag: string },
+): BareJsonGuardResult {
+  const trimmed = text.trim();
+  if (trimmed.length < 2) return { text, wasWrapped: false };
+
+  const first = trimmed[0];
+  if (first !== '{' && first !== '[') return { text, wasWrapped: false };
+
+  // Skip if the response already starts with a code fence that wraps the JSON.
+  if (/^```/.test(text.trimStart())) return { text, wasWrapped: false };
+
+  try {
+    JSON.parse(trimmed);
+  } catch {
+    return { text, wasWrapped: false };
+  }
+
+  logger.warn(
+    {
+      pathTag: context.pathTag,
+      length: text.length,
+      firstChar: first,
+      looksLikeArray: first === '[',
+    },
+    'Addie: Raw JSON envelope detected in outbound response — wrapping in code fence',
+  );
+
+  return {
+    text: '```json\n' + trimmed + '\n```',
+    wasWrapped: true,
+  };
+}
+
 /**
  * Extract markdown images from text and return them separately.
  * Used to convert markdown image syntax into Slack Block Kit image blocks,

server/src/addie/services/engagement-planner.ts

@@ -413,10 +413,13 @@ export async function composeMessage(
 
   text = stripCodeFences(text);
 
-  // Try to parse as JSON first for both Slack and email.
-  try {
-    const parsed = JSON.parse(text);
+  // Claude sometimes returns a JSON envelope followed by reflection text
+  // ("Wait, let me reconsider…") and then another envelope. JSON.parse rejects
+  // trailing content, so prefer the last well-formed top-level object —
+  // Claude's final decision after any reconsideration wins.
+  const parsed = parseEnvelope(text);
 
+  if (parsed) {
     if (parsed.skip) {
       logger.info(
         { person_id: ctx.relationship.id, reason: parsed.reason },
@@ -437,7 +440,7 @@ export async function composeMessage(
       };
     }
 
-    if (parsed.subject && parsed.body) {
+    if (parsed.subject && parsed.body && typeof parsed.subject === 'string' && typeof parsed.body === 'string') {
       const cleanedBody = extractUserFacingMessage(parsed.body, 'email');
       if (!cleanedBody) {
         logger.warn({ person_id: ctx.relationship.id }, 'Model returned non-user-facing email body');
@@ -450,8 +453,6 @@ export async function composeMessage(
         goalHint: inferGoalHint(cleanedBody, ctx.engagementOpportunities),
       };
     }
-  } catch {
-    // Not JSON — salvage the user-facing message text if possible.
   }
 
   const cleanedText = extractUserFacingMessage(text, channel);
@@ -481,6 +482,73 @@ function stripCodeFences(text: string): string {
   return codeFenceMatch ? codeFenceMatch[1].trim() : text;
 }
 
+/**
+ * Find all top-level `{...}` objects in the text and return the LAST one that
+ * parses as JSON. Covers the case where Claude emits an envelope, then a
+ * reflection paragraph, then a revised envelope — the revision is the answer.
+ *
+ * Uses a string/escape-aware bracket counter so quoted braces don't throw off
+ * the depth count.
+ */
+export function parseEnvelope(text: string): Record<string, unknown> | null {
+  const envelopes: Record<string, unknown>[] = [];
+  let cursor = 0;
+
+  while (cursor < text.length) {
+    const openIdx = text.indexOf('{', cursor);
+    if (openIdx === -1) break;
+
+    let depth = 0;
+    let inString = false;
+    let escape = false;
+    let closeIdx = -1;
+
+    for (let i = openIdx; i < text.length; i++) {
+      const c = text[i];
+      if (escape) { escape = false; continue; }
+      if (c === '\\') { escape = true; continue; }
+      if (c === '"') { inString = !inString; continue; }
+      if (inString) continue;
+      if (c === '{') depth++;
+      else if (c === '}') {
+        depth--;
+        if (depth === 0) { closeIdx = i; break; }
+      }
+    }
+
+    if (closeIdx === -1) break;
+
+    try {
+      const candidate = JSON.parse(text.slice(openIdx, closeIdx + 1));
+      if (candidate && typeof candidate === 'object' && !Array.isArray(candidate)) {
+        envelopes.push(candidate as Record<string, unknown>);
+      }
+    } catch {
+      // Not valid JSON at this boundary; keep scanning.
+    }
+
+    cursor = closeIdx + 1;
+  }
+
+  return envelopes.length > 0 ? envelopes[envelopes.length - 1] : null;
+}
+
+/**
+ * A paragraph that parses as a JSON object on its own — these are envelope
+ * fragments that leaked past `parseEnvelope` (e.g. missing/malformed closing
+ * brace would leave them intact). Defense in depth for the fallback path.
+ */
+function looksLikeJsonEnvelopeParagraph(paragraph: string): boolean {
+  const t = paragraph.trim();
+  if (!t.startsWith('{') || !t.endsWith('}')) return false;
+  try {
+    const p = JSON.parse(t);
+    return p && typeof p === 'object' && !Array.isArray(p);
+  } catch {
+    return false;
+  }
+}
+
 const REASONING_PREFIX_PATTERNS = [
   /^thinking about this one/i,
   /^thinking through this one/i,
@@ -570,7 +638,10 @@ export function extractUserFacingMessage(rawText: string, channel: 'slack' | 'em
   const paragraphs = text
     .split(/\n\s*\n/)
     .map(paragraph => paragraph.trim())
-    .filter(Boolean);
+    .filter(Boolean)
+    // Drop bare JSON envelope paragraphs — those are model scratch output that
+    // should never appear in a user-facing Slack or email message.
+    .filter(paragraph => !looksLikeJsonEnvelopeParagraph(paragraph));
 
   let firstUserFacingIndex = 0;
   while (firstUserFacingIndex < paragraphs.length && looksLikeReasoningParagraph(paragraphs[firstUserFacingIndex])) {

tests/addie/engagement-planner-envelope.test.ts

@@ -0,0 +1,100 @@
+import { describe, test, expect, vi } from 'vitest';
+
+vi.mock('../../server/src/logger.js', () => ({
+  logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
+  createLogger: vi.fn().mockReturnValue({
+    info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(),
+  }),
+}));
+
+import {
+  parseEnvelope,
+  extractUserFacingMessage,
+} from '../../server/src/addie/services/engagement-planner.js';
+
+describe('parseEnvelope', () => {
+  test('parses a clean JSON envelope', () => {
+    const out = parseEnvelope('{"text": "Hello"}');
+    expect(out).toEqual({ text: 'Hello' });
+  });
+
+  test('parses a JSON envelope with surrounding whitespace', () => {
+    const out = parseEnvelope('\n\n  {"text": "Hello"}  \n');
+    expect(out).toEqual({ text: 'Hello' });
+  });
+
+  test('picks the LAST valid envelope when model reconsiders', () => {
+    // Exact shape from production thread a88ea720 — Claude returned a draft,
+    // then a reflection paragraph, then a revised draft. The revised one
+    // (final decision after reconsideration) should win.
+    const raw =
+      '{"text": "Hey Luk — you joined a working group but I don\'t see any activity yet. Which one did you land in?"}\n\n' +
+      'Wait, let me reconsider — that violates rule #3 (open-ended question) and rule on unreplied (2 unreplied = lighter touch, pure value, no asks).\n\n' +
+      '{"text": "The agentic ads protocol docs just got a solid update — good reference if you\'re building in this space. Want me to drop the link?"}';
+
+    const out = parseEnvelope(raw);
+    expect(out).toBeDefined();
+    expect(out?.text).toContain('agentic ads protocol docs');
+    expect(out?.text).not.toContain('working group');
+  });
+
+  test('extracts JSON preceded by short reasoning prefix', () => {
+    const raw = 'Thinking about this one — lighter touch seems right.\n\n{"text": "Hope all is well!"}';
+    const out = parseEnvelope(raw);
+    expect(out).toEqual({ text: 'Hope all is well!' });
+  });
+
+  test('parses email envelopes with nested quoted braces', () => {
+    const out = parseEnvelope('{"subject": "Update", "body": "Your code: `if (x) {return}` — done."}');
+    expect(out?.subject).toBe('Update');
+    expect(out?.body).toContain('if (x)');
+  });
+
+  test('returns null on plain prose with no JSON object', () => {
+    expect(parseEnvelope('Just a regular message with no JSON.')).toBeNull();
+  });
+
+  test('returns null on malformed JSON with no valid top-level object', () => {
+    expect(parseEnvelope('{text: broken')).toBeNull();
+  });
+
+  test('handles escaped quotes inside strings without breaking bracket count', () => {
+    const out = parseEnvelope('{"text": "She said \\"hi\\" — so I replied."}');
+    expect(out?.text).toBe('She said "hi" — so I replied.');
+  });
+
+  test('handles braces inside strings without breaking bracket count', () => {
+    const out = parseEnvelope('{"text": "Use {placeholder} syntax for templates."}');
+    expect(out?.text).toBe('Use {placeholder} syntax for templates.');
+  });
+
+  test('passes over a {"skip": ...} control payload verbatim', () => {
+    const out = parseEnvelope('{"skip": true, "reason": "nothing meaningful to say"}');
+    expect(out).toEqual({ skip: true, reason: 'nothing meaningful to say' });
+  });
+});
+
+describe('extractUserFacingMessage — JSON envelope stripping', () => {
+  test('strips a bare JSON envelope paragraph that slipped through', () => {
+    // If a JSON envelope fragment reaches the fallback path, it should be
+    // dropped rather than sent to the user.
+    const raw = '{"text": "Hey there!"}\n\nAnd some follow-up prose that actually belongs in the message.';
+    const out = extractUserFacingMessage(raw, 'slack');
+    expect(out).toBe('And some follow-up prose that actually belongs in the message.');
+  });
+
+  test('leaves prose that merely starts with a brace untouched', () => {
+    const raw = '{Brace used stylistically} — this is a real user-facing message.';
+    const out = extractUserFacingMessage(raw, 'slack');
+    expect(out).toBe(raw);
+  });
+
+  test('returns null when only JSON envelopes and reasoning remain', () => {
+    const raw =
+      '{"text": "Draft one"}\n\n' +
+      'Thinking about this one — lighter touch seems right.\n\n' +
+      '{"text": "Draft two"}';
+    const out = extractUserFacingMessage(raw, 'slack');
+    expect(out).toBeNull();
+  });
+});